[WebKit-devel] RFC: API for KWebWallet...

[Dawit A.]

On Monday 09 November 2009 06:56:34 David Faure wrote:
> On Friday 06 November 2009, Dawit A. wrote:
> > It can be implemented using QFuture. It would simply mean that this class
> > defers the decision and the work of whether or not the call should be
> > async or  not to the caller itself. In other words, the caller can choose
> > to call result() on what the saveFormData function returns and block
> > waiting for the result or it can choose to use QFutureWatcher and become
> > non-blocking. Of course, QFutureWatcher uses signals/slots.
> 
> I understand the API idea, but I don't understand how you can implement it.
> 
> What will you write in the implementation of
> QFuture<bool> KWebWallet::saveFormData(QWebFrame* frame)
> {
>   QFuture<bool> myfuture;
>   // what here ???
>   return myfuture;
> }

// code to extract and store form data in a container...
myfuture = QtConcurrent::run(doSaveFormData, ...);

Note that the current signature of "doSaveFormData" member function is 
actually not the final one. It is simply a place holder right now. That is 
because whether or not a QFuture solution is used, the actual form data will 
have to be extracted and stored in the saveDataForm function. Otherwise, the 
page data might change from underneath us and we will be either saving 
incorrect data or nothing at all. So the non-blocking portion of this is only 
saving the extracted data into the wallet itself.

> QFuture has no virtual methods, and the only thing that seems to create
> QFutures in Qt is QtConcurrent::run. So what would you write in there,
> that doesn't use a thread?

Do not understand this question but my remarks above hopefully answer it...

> Either I'm missing something (and then I apologize), or this whole idea
> does not work, because QFuture is only meant for threads.
>
> And if we're not talking about threads, then implementing "blocking on top
>  of async" means nested event loop, which means unexpected reentrancy,
>  which means bugs and crashes, which I don't really recommend.

Well if we were going to use the QFuture approach, the original API I 
presented would have to have been modified accordingly. For example, the 
saveFormDataRequested signal would have probably been replaced with a internal 
virtual function that provided some sort of default implementation.

Anyhow, I am going to stick to the original API for the reasons I mentioned in 
the previous response. We can always evaluate if we need to modify that 
implementation afterwards...