Bug 173362 - Add API::InjectedBundle::ResourceLoadClient
Summary: Add API::InjectedBundle::ResourceLoadClient
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 173383
Blocks: 173364
  Show dependency treegraph
 
Reported: 2017-06-14 05:11 PDT by Carlos Garcia Campos
Modified: 2017-06-15 01:20 PDT (History)
4 users (show)

See Also:


Attachments
Patch (30.78 KB, patch)
2017-06-14 05:14 PDT, Carlos Garcia Campos
achristensen: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-06-14 05:11:46 PDT
It will be used by the GTK+ port instead of the C API. This is the last one we need to get rid of the C API in the GTK+ implementation.
Comment 1 Carlos Garcia Campos 2017-06-14 05:14:44 PDT
Created attachment 312884 [details]
Patch
Comment 2 Alex Christensen 2017-06-14 08:45:25 PDT
Comment on attachment 312884 [details]
Patch

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

> Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:336
> -    page.initializeInjectedBundleResourceLoadClient(&client.base);
> +    WKBundlePageSetResourceLoadClient(toAPI(&page), &client.base);

I'm not sure about this change.  From API objects I think we want to call C++ methods, not use the C API.

> Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:353
> -        _page->initializeInjectedBundleResourceLoadClient(nullptr);
> +        WKBundlePageSetResourceLoadClient(toAPI(_page.get()), nullptr);

ditto
Comment 3 Carlos Garcia Campos 2017-06-14 08:51:19 PDT
(In reply to Alex Christensen from comment #2)
> Comment on attachment 312884 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=312884&action=review
> 
> > Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:336
> > -    page.initializeInjectedBundleResourceLoadClient(&client.base);
> > +    WKBundlePageSetResourceLoadClient(toAPI(&page), &client.base);
> 
> I'm not sure about this change.  From API objects I think we want to call
> C++ methods, not use the C API.

But the C API is used there, mixing both is what caused this. If you prefer to use the C++ API, then this should be migrated to use API::InjectedBundle::ResourceLoadClient the same way we are doing for all our clients in the GTK+ API.

> > Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:353
> > -        _page->initializeInjectedBundleResourceLoadClient(nullptr);
> > +        WKBundlePageSetResourceLoadClient(toAPI(_page.get()), nullptr);
> 
> ditto

Thanks for all the reviews, btw!
Comment 4 Carlos Garcia Campos 2017-06-14 10:10:39 PDT
Committed r218266: <http://trac.webkit.org/changeset/218266>
Comment 5 WebKit Commit Bot 2017-06-14 13:49:31 PDT
Re-opened since this is blocked by bug 173383
Comment 6 Carlos Garcia Campos 2017-06-15 01:20:45 PDT
Committed r218321: <http://trac.webkit.org/changeset/218321>