Summary: | [GTK] Add resources API to WebKit2 GTK+ | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, gustavo, pnormand | ||||||
Priority: | P2 | Keywords: | Gtk | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Linux | ||||||||
Bug Depends on: | 79471 | ||||||||
Bug Blocks: | 79667 | ||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2012-02-24 04:57:51 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 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 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 (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 Created attachment 132090 [details]
Updated patch
Just rebased to apply on current git master and fixed the typo found by phil.
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 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 ;) (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 (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. Ah, it's marked WONTFIX, that resolved through me off =( (In reply to comment #10) > Ah, it's marked WONTFIX, that resolved through me off =( Yes, my fix will actually be a workaround. Committed r112221: <http://trac.webkit.org/changeset/112221> |