[Webkit-unassigned] [Bug 24529] WebInspector: HTML5 Offline Web Applications Support (ApplicationCache)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 8 19:00:13 PDT 2010


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





--- Comment #77 from Joseph Pecoraro <joepeck at webkit.org>  2010-07-08 19:00:12 PST ---
(From update of attachment 60992)
> +                'public/WebApplicationCacheResourceInfoList.h'

I don't see this file. I saw it in an older patch. Is it being added?


> +++ WebKit/chromium/src/ApplicationCacheHost.cpp	(working copy)
> +#include "InspectorApplicationCacheAgent.h"

Hmm, I had this in my starter patch but I don't think its needed. This
includes ApplicationCacheHost.h which includes that header. You may be
able to remove it.


> +        for (size_t i = 0; i < webResources.size(); ++i) {
> +        // Convert results from webResources to resources.
> +        resources->append(InspectorApplicationCacheAgent::ResourceInfo(
> +            KURL(webResources[i].m_resource), webResources[i].m_isMaster, webResources[i].m_isManifest, webResources[i].m_isFallback,
> +                 webResources[i].m_isForeign, webResources[i].m_isExplicit, webResources[i].m_size));
> +        }

Style: Indent the contents of a for loop. So the comment and resources->append statements.


> +++ WebKit/chromium/ChangeLog	(working copy)
> @@ -1,3 +1,19 @@
> +2010-07-08  Kavita Kanetkar  <kkanetkar at chromium.org>
> +        (WebCore::ApplicationCacheHost::applicationCacheInfo): Unimplimented.

Typo: "Unimplimented" => "Unimplemented"


> +++ WebCore/ChangeLog	(working copy)
> +        No new tests. (OOPS!)

This line can be removed, or expanded to say nothing has changed.
Might be worth adding a comment as well. Like "Moving functions
from InspectorApplicationCacheAgent to ApplicationCacheHost to
make it easier for Chromium to provide implementations."


> +++ WebCore/inspector/front-end/ApplicationCacheItemsView.js	(working copy)
> @@ -139,7 +139,8 @@
>          this._resources = applicationCaches.resources;
>          var lastPathComponent = applicationCaches.lastPathComponent;
>  
> -        if (!this._manifest) {
> +        // FIXME: Change this to check manifest instead of resources.
> +        if (!this._resources.length) {

Yes, this FIXME is important because this makes it so a valid manifest without
resources will show the "No AppCache" message. Any idea what it will take
to include an implementation of applicationCacheInfo()? It was in your older
patch.


> +++ WebCore/loader/appcache/ApplicationCacheHost.cpp	(working copy)
> +InspectorApplicationCacheAgent::ApplicationCacheInfo ApplicationCacheHost::applicationCacheInfo()
> +{
> +    ApplicationCache* cache = applicationCache();
> +        if (!cache || !cache->isComplete())
> +            return InspectorApplicationCacheAgent::ApplicationCacheInfo(KURL(), String(), String(), 0);

Style: If statement and body are double indented.

This is another weird whitespace regression in one of your patches =).
Any idea why the formatting changed?


> +++ WebCore/loader/appcache/ApplicationCacheHost.h	(working copy)
>  #if ENABLE(OFFLINE_WEB_APPLICATIONS)
> -
> +#include "InspectorApplicationCacheAgent.h"

Style: I think we want to leave that blank line in.


>          void stopDeferringEvents(); // Also raises the events that have been queued up.
> +#if ENABLE(INSPECTOR)
> +        void fillResourceList(InspectorApplicationCacheAgent::ResourceInfoList*);
> +        InspectorApplicationCacheAgent::ApplicationCacheInfo applicationCacheInfo();
> +#endif

Style: Same here, I think there should be a blank line before and the #if.

-- 
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