[Qt][WK2] Keep the WebContext alive during the whole application's lifetime

[Jocelyn Turcotte]

[Qt][WK2] Keep the WebContext alive during the whole application's lifetime

[Jocelyn Turcotte]

Created attachment 188341 [details]
Patch

[Simon Hausmann]

Comment on attachment 188341 [details]
Patch

Patch looks good to me. Benjamin, can you sign off on this patch? Thank you :)

[Simon Hausmann]

WK2-sign-off-ping :)

[Benjamin Poulain]

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?

[Jocelyn Turcotte]

(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!

[Jocelyn Turcotte]

Committed r145513: <http://trac.webkit.org/changeset/145513>