[webkit-reviews] review denied: [Bug 56415] ApplicationCacheGroup is not obsolete after being deleted via ApplicationCacheStorage::deleteEntriesForOrigin : [Attachment 85883] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Mar 15 17:49:24 PDT 2011
Alexey Proskuryakov <ap at webkit.org> has denied Anton D'Auria
<adauria at apple.com>'s request for review:
Bug 56415: ApplicationCacheGroup is not obsolete after being deleted via
ApplicationCacheStorage::deleteEntriesForOrigin
https://bugs.webkit.org/show_bug.cgi?id=56415
Attachment 85883: Patch
https://bugs.webkit.org/attachment.cgi?id=85883&action=review
------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=85883&action=review
I don't think that the approach here is safe. To forget a used application
cache, you should call ApplicationCacheGroup::makeObsolete() -
ApplicationCacheStorage::deleteCacheGroup() is just untested and unused code
that has been landed by mistake, as far as I can tell.
It's not good to manipulate ApplicationCacheStorage directly behind the back of
an existing ApplicationCacheGroup.
> Source/WebCore/ChangeLog:7
> + ApplicationCacheGroup is not obsolete after being deleted via
ApplicationCacheStorage::deleteEntriesForOrigin
> +
> + https://bugs.webkit.org/show_bug.cgi?id=56415
We don't usually put a blank line between bug title and number.
> Source/WebKit/mac/ChangeLog:7
> + ApplicationCacheGroup is not obsolete after being deleted via
ApplicationCacheStorage::deleteEntriesForOrigin
> +
> + https://bugs.webkit.org/show_bug.cgi?id=56415
Extra blank line, again.
> Tools/DumpRenderTree/LayoutTestController.cpp:393
> + JSRetainPtr<JSStringRef> originUrl(Adopt, JSValueToStringCopy(context,
arguments[0], exception));
"URL" should be capitalized.
More information about the webkit-reviews
mailing list