[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


Joseph Pecoraro <joepeck at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #62248|                            |review-
               Flag|                            |




--- Comment #18 from Joseph Pecoraro <joepeck at webkit.org>  2010-07-21 17:30:07 PST ---
(From update of attachment 62248)
> +++ WebKit/chromium/src/ApplicationCacheHost.cpp	(working copy)
> +    for (int i = 0; i < webResources.size(); ++i) {
> +        resources->append(ResourceInfo(
> +        KURL(webResources[i].url), webResources[i].isMaster, webResources[i].isManifest, webResources[i].isFallback, webResources[i].isForeign, webResources[i].isExplicit, webResources[i].size));
> +        }

Whitespace issue.


> +++ WebCore/inspector/InspectorApplicationCacheAgent.h	(working copy)
>  #include "ApplicationCacheHost.h"
> -#include "KURL.h"
>  #include "PlatformString.h"
>  #include <wtf/Noncopyable.h>

Can PlatformString be removed here as well? I don't see other String usage.


>      // Backend to Frontend
>      void didReceiveManifestResponse(unsigned long identifier, const ResourceResponse&);
> +
>      void updateApplicationCacheStatus(ApplicationCacheHost::Status);
> +
>      void updateNetworkState(bool isNowOnline);

Why the extra newlines?


> +++ WebCore/loader/appcache/ApplicationCacheHost.h	(working copy)
> +#include "KURL.h"
>      ...
>      class KURL;

Looks like the forward declaration can be removed.


> +        struct CacheInfo {
> +            CacheInfo(const KURL& manifest, double creationTime, double updateTime, long long size)
> +                : m_manifest(manifest)
> +                , m_creationTime(creationTime)
> +                , m_updateTime(updateTime)
> +                , m_size(size)
> +        {
> +        }
> +
> +        KURL m_manifest;
> +        double m_creationTime;
> +        double m_updateTime;
> +        long long m_size;
> +    };

Whitespace issues again on the "struct" and constructor lines.

You did change the creation and update times from String to double.
I like that change. I think it would be nice to clarify this in the
ChangeLog. You didn't just "move it" you also changed it.


> +        struct CacheInfo {
> +    struct ResourceInfo {
> +    typedef Vector<ResourceInfo> ResourceInfoList;

I think these should be moved into the ENABLE(INSPECTOR) guard.

I don't think any of these are r- worthy, but you don't have committer
rights yet. The Chromium side still needs a chromium reviewer, but it looks
straightforward.

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