[Webkit-unassigned] [Bug 41993] WebKit API for WebInspector-Appcache integration for chrome.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 14 16:58:13 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=41993





--- Comment #3 from Michael Nordman <michaeln at google.com>  2010-07-14 16:58:13 PST ---
Thnx for looking.

(In reply to comment #2)
> (From update of attachment 61568 [details])
> WebKit/chromium/public/WebApplicationCacheHost.h:96
>  +      struct ApplicationCacheInfo {
> this struct is contained in the WebApplicationCacheHost "namespace"
> so the ApplicationCache prefix here is a bit redundant.  however,
> just calling it Info seems too vague.  maybe CacheInfo is enough
> since you use the method name getAssociatedCacheInfo?

CacheInfo it is!

> WebKit/chromium/public/WebApplicationCacheHost.h:97
>  +          WebURL manifestUrl; // Empty if there is no associated cache.
> nit: manifestUrl -> manifestURL

will do

> WebKit/chromium/public/WebApplicationCacheHost.h:113
>  +      // FIXME: Make these pure virtual after chrome has caught up.
> Actually, our convention for WebKit API interfaces that are
> implemented by the embedder is that they should not have pure
> virtual methods.

hmmm... then i suppose i should change all the others as well (yes?)

> WebKit/chromium/public/WebApplicationCacheHostClient.h:43
>  +      virtual void didChangeCacheAssociation() { return; } // FIXME: Make this pure virtual.
> this one can be pure virtual now since you can provide a dummy
> implementation in ApplicationCacheHostInternal.h.

ok, will do

> WebKit/chromium/public/WebApplicationCacheHost.h:114
>  +      virtual void getAssociatedCacheInfo(ApplicationCacheInfo*) { return; }
> style-nit: no need for the return statements

duh

> WebKit/chromium/public/WebApplicationCacheHost.h:116
>  +      virtual void deleteAssociatedCacheGroup() { return; }
> it is unclear to me why this is called deleteAssociatedCacheGroup
> and not deleteAssociatedCache given the method named
> getAssociatedCacheInfo.

Because the action will be to delete everything related to the manifestURL. In appcache speak, that's an ApplicationCacheGroup. I'd be ok with using deleteAssociatedCache shorthand here, but really the addition of "Group" is more descriptive.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list