[Webkit-unassigned] [Bug 42426] Implement remaining Inspector support for chrome's appcache

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Michael Nordman <michaeln at google.com>  2010-07-16 12:26:33 PST ---
(From update of attachment 61760)


WebKit/chromium/src/ApplicationCacheHost.cpp:213
 +  InspectorApplicationCacheAgent::ApplicationCacheInfo ApplicationCacheHost::applicationCacheInfo()
this outgoing call needs a check for m_internal

WebKit/chromium/src/ApplicationCacheHost.cpp:217
 +      // Convert to webcore.
i think the comment about converting here (and below) state the obvious and aren't really needed

WebKit/chromium/src/ApplicationCacheHost.cpp:229
 +              KURL(webResources[i].url), webResources[i].isMaster, webResources[i].isManifest, webResources[i].isFallback, webResources[i].isForeign,
indent is off here

class WebURL has a KURL() operator so i don't think we need to explicitly invoke the KURL ctor

WebKit/chromium/src/ApplicationCacheHostInternal.h:58
 +          // FIXME: Prod the inspector to update it's notion of what cache the page is using.
compile bustage... duplicate method


WebKit/chromium/src/ApplicationCacheHostInternal.h: 
 +  
any reason to remove this blank line?

WebCore/inspector/InspectorApplicationCacheAgent.cpp:55
 +  void InspectorApplicationCacheAgent::updateApplicationCacheStatus(int status)
If it's technically impossible to use the enum, maybe a comment to make it clear what values are expected here.
 // Status is of type ApplicationCacheHost::Status

Although for the life of me I can't see where its technically impossible to use the more descriptive type. I don't see what the issue is with defining the ResourceInfo and CacheInfo structs in the ApplicationCacheHost.h file. They are part of the interface (not the implementation) and just belong there. 

Here's how this same thing looks on the class that represents ApplicationCacheHost in chromium's WebKitAPI.
http://trac.webkit.org/browser/trunk/WebKit/chromium/public/WebApplicationCacheHost.h

95        // Structures and methods to support inspecting Application Caches.
96        struct CacheInfo {
97            WebURL manifestURL; // Empty if there is no associated cache.
98            double creationTime;
99            double updateTime;
100            long long totalSize;
101            CacheInfo() : creationTime(0), updateTime(0), totalSize(0) { }
102        };
103        struct ResourceInfo {
104            WebURL url;
105            long long size;
106            bool isMaster;
107            bool isManifest;
108            bool isExplicit;
109            bool isForeign;
110            bool isFallback;
111            ResourceInfo() : size(0), isMaster(false), isManifest(false), isExplicit(false), isForeign(false), isFallback(false) { }
112        };
113        virtual void getAssociatedCacheInfo(CacheInfo*) { }
114        virtual void getResourceList(WebVector<ResourceInfo>*) { }
115        virtual void deleteAssociatedCacheGroup() { }

I'd like to see us that the corresponding thing in webcore/loader/appcache/ApplicationCacheHost.h. Then cyclic .h dependencies just go away.

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