Bug 79477 - [GTK] Add resources API to WebKit2 GTK+
Summary: [GTK] Add resources API to WebKit2 GTK+
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on: 79471
Blocks: 79667
  Show dependency treegraph
 
Reported: 2012-02-24 04:57 PST by Carlos Garcia Campos
Modified: 2012-03-27 01:00 PDT (History)
3 users (show)

See Also:


Attachments
Patch (69.58 KB, patch)
2012-02-24 05:24 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (68.45 KB, patch)
2012-03-15 11:41 PDT, Carlos Garcia Campos
gns: 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 2012-02-24 04:57:51 PST
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.
Comment 1 Carlos Garcia Campos 2012-02-24 05:24:18 PST
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
Comment 2 WebKit Commit Bot 2012-02-24 05:25:32 PST
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
Comment 3 Philippe Normand 2012-03-15 01:19:26 PDT
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
Comment 4 Carlos Garcia Campos 2012-03-15 01:35:49 PDT
(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
Comment 5 Carlos Garcia Campos 2012-03-15 11:41:12 PDT
Created attachment 132090 [details]
Updated patch

Just rebased to apply on current git master and fixed the typo found by phil.
Comment 6 Gustavo Noronha (kov) 2012-03-26 09:05:29 PDT
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)
Comment 7 Gustavo Noronha (kov) 2012-03-26 09:07:20 PDT
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 ;)
Comment 8 Carlos Garcia Campos 2012-03-26 09:12:42 PDT
(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
Comment 9 Carlos Garcia Campos 2012-03-26 09:13:58 PDT
(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.
Comment 10 Gustavo Noronha (kov) 2012-03-26 10:45:02 PDT
Ah, it's marked WONTFIX, that resolved through me off =(
Comment 11 Carlos Garcia Campos 2012-03-26 10:59:24 PDT
(In reply to comment #10)
> Ah, it's marked WONTFIX, that resolved through me off =(

Yes, my fix will actually be a workaround.
Comment 12 Carlos Garcia Campos 2012-03-27 01:00:21 PDT
Committed r112221: <http://trac.webkit.org/changeset/112221>