Bug 202081 – Require a WebsiteDataStore when creating or resuming downloads

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Move DownloadClient from WebProcessPool to WebsiteDataStore

[Alex Christensen]

Created attachment 379324 [details]
Patch

[Alex Christensen]

I'm assuming this will break some API that GTK already has.  That API should be deprecated and replaced by one on WebsiteDataStore.

[Alex Christensen]

Created attachment 379325 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Alex Christensen]

Created attachment 379326 [details]
Patch

[Alex Christensen]

Created attachment 379327 [details]
Patch

[Alex Christensen]

Created attachment 379328 [details]
Patch

[Alex Christensen]

Created attachment 379329 [details]
Patch

[Alex Christensen]

iOS test failure unrelated.  This is ready for review.

[Alex Christensen]

Comment on attachment 379329 [details]
Patch

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

> Source/WebKit/UIProcess/API/C/WKContext.h:116
> +WK_EXPORT void WKContextSetDownloadClient(WKContextRef context, const WKContextDownloadClientBase* client) WK_C_API_DEPRECATED_WITH_REPLACEMENT(WKWebsiteDataStoreSetDownloadClient);

Not quite ready.  I need to shim WKContextSetDownloadClient onto WKWebsiteDataStoreSetDownloadClient for an internal client during the transition.

[Alex Christensen]

Created attachment 379397 [details]
Patch

[Alex Christensen]

Created attachment 379409 [details]
Patch

[Alex Christensen]

Created attachment 379410 [details]
Patch

[Alex Christensen]

Created attachment 379414 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 379414 [details]
Patch

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

> Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:1661
> -    return webkitWebContextGetOrCreateDownload(&context->priv->processPool->download(initiatingPage, request));
> +    return webkitWebContextGetOrCreateDownload(&context->priv->processPool->download(WebKit::WebsiteDataStore::defaultDataStore().get(), initiatingPage, request));

We never use the default data store in the GLib API. Here we should use the web context data store. So this should be:

return webkitWebContextGetOrCreateDownload(&context->priv->processPool->download(webkitWebsiteDataManagerGetDataStore(context->priv->websiteDataManager.get()), initiatingPage, request));

[youenn fablet]

Comment on attachment 379414 [details]
Patch

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

> Source/WebKit/UIProcess/WebProcessPool.cpp:1330
> +    PAL::SessionID sessionID = dataStore.sessionID();

auto

> Source/WebKit/UIProcess/WebProcessPool.cpp:1362
> +    PAL::SessionID sessionID = dataStore.sessionID();

auto

> Source/WebKit/UIProcess/API/C/WKContext.cpp:175
> +        explicit DownloadClient(const WKContextDownloadClientBase* client, WKContextRef context)

no need for explicit anymore

> Source/WebKit/UIProcess/API/glib/WebKitDownloadClient.cpp:43
> +    void didStart(DownloadProxy& downloadProxy) override

final here and below.

> Source/WebKit/UIProcess/Downloads/DownloadProxy.h:60
> +    static Ref<DownloadProxy> create(DownloadProxyMap&, WebsiteDataStore&, WebProcessPool&, const WebCore::ResourceRequest&);

Could take Ref<WebsiteDataStore>&&, ditto for WebProcessPool.

> Source/WebKit/UIProcess/Downloads/DownloadProxy.h:103
> +    explicit DownloadProxy(DownloadProxyMap&, WebsiteDataStore&, WebProcessPool&, const WebCore::ResourceRequest&);

No need of explicit.

[Alex Christensen]

(In reply to Carlos Garcia Campos from comment #15)
> Comment on attachment 379414 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=379414&action=review
> 
> > Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:1661
> > -    return webkitWebContextGetOrCreateDownload(&context->priv->processPool->download(initiatingPage, request));
> > +    return webkitWebContextGetOrCreateDownload(&context->priv->processPool->download(WebKit::WebsiteDataStore::defaultDataStore().get(), initiatingPage, request));
> 
> We never use the default data store in the GLib API. Here we should use the
> web context data store. So this should be:
> 
> return
> webkitWebContextGetOrCreateDownload(&context->priv->processPool-
> >download(webkitWebsiteDataManagerGetDataStore(context->priv-
> >websiteDataManager.get()), initiatingPage, request));

I'm about to remove the context's WebsiteDataStore.

[Alex Christensen]

http://trac.webkit.org/r250292

[Radar WebKit Bug Importer]

<rdar://problem/55661326>