Bug 109827

Summary: [Qt][WK2] Keep the WebContext alive during the whole application's lifetime
Product: WebKit Reporter: Jocelyn Turcotte <jturcotte>
Component: New BugsAssignee: Jocelyn Turcotte <jturcotte>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, hausmann, jturcotte
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 109829    
Attachments:
Description Flags
Patch hausmann: review+

Description Jocelyn Turcotte 2013-02-14 06:55:19 PST
[Qt][WK2] Keep the WebContext alive during the whole application's lifetime
Comment 1 Jocelyn Turcotte 2013-02-14 07:00:58 PST
Created attachment 188341 [details]
Patch
Comment 2 Simon Hausmann 2013-02-20 06:58:32 PST
Comment on attachment 188341 [details]
Patch

Patch looks good to me. Benjamin, can you sign off on this patch? Thank you :)
Comment 3 Simon Hausmann 2013-02-27 01:00:00 PST
WK2-sign-off-ping :)
Comment 4 Benjamin Poulain 2013-02-27 12:45:20 PST
Comment on attachment 188341 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188341&action=review

I have no problem with this landing.

Why do you have issues with the lifetime of WebContext? Is it because you want to keep a global shared Context?

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:123
>  QtWebContext::~QtWebContext()
>  {
> -    ASSERT(!s_defaultQtWebContext || s_defaultQtWebContext == this);
> -    s_defaultQtWebContext = 0;
>  }

Can't you remove the destructor entirely?

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:135
> +        RefPtr<WebContext> webContext = WebContext::create(String());

WKContextCreate?
Comment 5 Jocelyn Turcotte 2013-02-28 01:36:22 PST
(In reply to comment #4)
> Why do you have issues with the lifetime of WebContext? Is it because you want to keep a global shared Context?

The main issue I had was when trying to open/close the IconDatabase quickly in tests.
IconDatabase::dispatchDidFinishURLImportOnMainThread would post a callback on the main thread, but by the time it's run there the object would be deleted already. Since it uses a temporary work item object with no reference elsewhere I can't fix it by calling cancelCallOnMainThread.

On top of that, internal references are kept to the WebContext by WebProcessProxies, which only release it after their web process terminated, 60 seconds after the last page closed. To keep it that way I would also need some callback in the C API to know when all internal references have been dropped to know when I can destroy my wrapper objects.

So for the moment it's simpler and more reliable to keep it alive.

> 
> > Source/WebKit2/UIProcess/qt/QtWebContext.cpp:123
> >  QtWebContext::~QtWebContext()
> >  {
> > -    ASSERT(!s_defaultQtWebContext || s_defaultQtWebContext == this);
> > -    s_defaultQtWebContext = 0;
> >  }
> 
> Can't you remove the destructor entirely?

Yes I might as well.

> 
> > Source/WebKit2/UIProcess/qt/QtWebContext.cpp:135
> > +        RefPtr<WebContext> webContext = WebContext::create(String());
> 
> WKContextCreate?

It's done later in bug 108475.

Thanks!
Comment 6 Jocelyn Turcotte 2013-03-12 04:14:49 PDT
Committed r145513: <http://trac.webkit.org/changeset/145513>