Bug 79477 – [GTK] Add resources API to WebKit2 GTK+

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Carlos Garcia Campos]

As discussed in the mailing list we should provide a WebResource object and methods to get the main resources and all subresources from a WebView.

[Carlos Garcia Campos]

Created attachment 128717 [details]
Patch

This is an initial patch to add resources API. There are some things still to do that I'm leaving for following patches to make this one a bit smaller.

 - Ignore resources when replacing content
 - Workaround for bug #78510.
 - API to save resources
 - API to save a web view

[WebKit Commit Bot]

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

[Philippe Normand]

Comment on attachment 128717 [details]
Patch

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

Patch looks good to me but I think another pair of eyes needs to look at it :)

> Source/WebKit2/UIProcess/API/gtk/tests/TestResources.cpp:412
> +static void addCacheHTTPHeadersToRespose(SoupMessage* message)

typo: addCacheHTTPHeadersToResponse

[Carlos Garcia Campos]

(In reply to comment #3)
> (From update of attachment 128717 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=128717&action=review
> 
> Patch looks good to me but I think another pair of eyes needs to look at it :)

Thanks for reviewing it. Gustavo? :-P

> > Source/WebKit2/UIProcess/API/gtk/tests/TestResources.cpp:412
> > +static void addCacheHTTPHeadersToRespose(SoupMessage* message)
> 
> typo: addCacheHTTPHeadersToResponse

Oops

[Carlos Garcia Campos]

Created attachment 132090 [details]
Updated patch

Just rebased to apply on current git master and fixed the typo found by phil.

[Gustavo Noronha (kov)]

Comment on attachment 132090 [details]
Updated patch

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

Looks good to me!

> Source/WebKit2/UIProcess/API/gtk/WebKitWebResource.cpp:263
> + *   is emitted again with a redirected response, the active URI is the
> + *   redirected URI.

"the active URI is the URI the request was redirected to" sounds better I think

> Source/WebKit2/UIProcess/API/gtk/tests/TestResources.cpp:300
> +    // No cached resurce: First load.

s/resurce/resource/ (and a few more after this)

[Gustavo Noronha (kov)]

Comment on attachment 132090 [details]
Updated patch

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

> Source/WebKit2/UIProcess/API/gtk/tests/TestResources.cpp:151
> +#if 0
> +    // Load the same URI again.
> +    // FIXME: we need a workaround for bug https://bugs.webkit.org/show_bug.cgi?id=78510.
> +    test->loadURI(kServer->getURIForPath("/").data());
> +    test->waitUntilResourcesLoaded(4);
> +#endif

You're probably more aware than I am of this, but worth pointing out that you fixed 78510 already ;)

[Carlos Garcia Campos]

(In reply to comment #6)
> (From update of attachment 132090 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=132090&action=review
> 
> Looks good to me!

Thank you very much for reviewing :-)

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebResource.cpp:263
> > + *   is emitted again with a redirected response, the active URI is the
> > + *   redirected URI.
> 
> "the active URI is the URI the request was redirected to" sounds better I think

Indeed.

> > Source/WebKit2/UIProcess/API/gtk/tests/TestResources.cpp:300
> > +    // No cached resurce: First load.
> 
> s/resurce/resource/ (and a few more after this)

Ooops :-P

[Carlos Garcia Campos]

(In reply to comment #7)
> (From update of attachment 132090 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=132090&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/tests/TestResources.cpp:151
> > +#if 0
> > +    // Load the same URI again.
> > +    // FIXME: we need a workaround for bug https://bugs.webkit.org/show_bug.cgi?id=78510.
> > +    test->loadURI(kServer->getURIForPath("/").data());
> > +    test->waitUntilResourcesLoaded(4);
> > +#endif
> 
> You're probably more aware than I am of this, but worth pointing out that you fixed 78510 already ;)

No I didn't :-( I was waiting to land the resources patches before trying a workaround for that bug.

[Gustavo Noronha (kov)]

Ah, it's marked WONTFIX, that resolved through me off =(

[Carlos Garcia Campos]

(In reply to comment #10)
> Ah, it's marked WONTFIX, that resolved through me off =(

Yes, my fix will actually be a workaround.

[Carlos Garcia Campos]

Committed r112221: <http://trac.webkit.org/changeset/112221>