[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