Summary: | [Chromium] Need to add manifestUrl to ResourceResponse. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Nordman <michaeln> | ||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | eric, fishd | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Michael Nordman
2009-09-03 18:28:28 PDT
Created attachment 39027 [details]
simple patch
Also fixes a NPE in our v8 bindings.
These seem like two unrelated changes. (In reply to comment #2) > These seem like two unrelated changes. I guess the common thread is chrome/appcache. How do other implementations solve the same "problem"? I presume your wondering about webcore's impl... see ApplicationCacheHost::selectCacheWithManifest and ApplicationCacheGroup::selectCache. These methods detect 'foreign' entries and as a side effect may restart the frame navigation. Also see WebAppplicationCacheHostImpl::selectCacheWithManifest() in chrome code. We solve the "problem" in the same way. We just get the manifestUrl the response was loaded from in a different way. fyi: here's the chrome-side of this http://codereview.chromium.org/201026/show Comment on attachment 39027 [details] simple patch > Index: WebCore/ChangeLog ... > +2009-08-31 Michael Nordman <michaeln@google.com> > + > + Reviewed by NOBODY (OOPS!). > + > + Chromium appcache modifications. > + > + No new tests. nit: please include a link to this bug report. > Index: WebCore/bindings/v8/custom/V8DOMApplicationCacheCustom.cpp ... > + if (EventListener* listener = appcache->getAttributeEventListener(toEventID(name))) > + return eventListenerToV8Object(listener); nit: indentation should be 4 white spaces > Index: WebCore/platform/network/chromium/ResourceResponse.h ... > + const KURL& getAppCacheManifestURL() const { return m_appCacheManifestURL; } getters should not start with "get"... just call this appCacheManifestURL. Created attachment 39127 [details]
simple patch 2
(OOPS!)
Also made changes the other existing getters/setters to match naming and style.
The matching change in chrome has been updated as well... http://codereview.chromium.org/201026/show If I understand correctly, the DEPRECATED members exist so we can avoid a two-sided landing. Hence, I went ahead and set commit-queue+. Comment on attachment 39127 [details] simple patch 2 Clearing flags on attachment: 39127 Committed r48109: <http://trac.webkit.org/changeset/48109> All reviewed patches have been landed. Closing bug. Created attachment 39202 [details]
house cleaning
Remove a few deprecated methods.
Resetting status to UNCONFIRMED so the new house cleaning patch is seen. (Not sure if thats actually necessary or not for the patch to be picked up by std queries for things todo?) Bitten by bug 28230. Comment on attachment 39202 [details]
house cleaning
Given no indication otherwise, I assume this patch can be landed w/o coordination with Chromium. r+/cq+
(In reply to comment #17) > (From update of attachment 39202 [details]) > Given no indication otherwise, I assume this patch can be landed w/o > coordination with Chromium. r+/cq+ Yes, thank you! Comment on attachment 39202 [details] house cleaning Clearing flags on attachment: 39202 Committed r48175: <http://trac.webkit.org/changeset/48175> All reviewed patches have been landed. Closing bug. |