[Webkit-unassigned] [Bug 24529] Add support in Web Inspector for examining/changing data for HTML5 offline-Web-applications (application cache)

[bugzilla-daemon at webkit.org]

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


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54234|review?                     |review-
               Flag|                            |




--- Comment #22 from Pavel Feldman <pfeldman at chromium.org>  2010-04-25 05:17:26 PST ---
(From update of attachment 54234)
WebCore/inspector/InspectorApplicationCacheAgent.cpp:38
 +  : m_inspectorController(inspectorController)
We used to indent these. Did it change?

WebCore/inspector/InspectorApplicationCacheAgent.cpp:54
 +      if (m_frontend)
You should limit the lifetime of this agent to the lifetime of the front-end.
You would not need to check for frontend existence then. In fact I don't see
where you clear this field, so the check does not work anyways.

WebCore/inspector/InspectorController.cpp:509
 +      m_applicationCacheAgent = 0;
This should instead be in the disconnectFrontend. Lifetime of this agent is
similar to the InspectorDOMAgent. You could further generalize releaseDOMAgent
to releaseAgents and do the tear down there. That way you can safely reference
front-end from within the agent.

WebCore/inspector/InspectorController.h:192
 +      void getCookies(long callId);
(now that you touched it - it should have been called cookies back then (and
the other guy rawCookies) with no get prefix.)

WebCore/inspector/front-end/ApplicationCacheItemsView.js:74
 +              this.connectivityIcon, this.connectivityMessage, this.divider,
[Online] [Idle] confuses me on the second screenshot. Should one of the
statuses go to the tree view or to the table header?

WebCore/inspector/front-end/StoragePanel.js:558
 +          // as a localized string. Should it be the name of the Manifest
file? Domain?
We should distinguish these by domain when we have multiple iframes loaded,
right? So it should be just domain name under the Applications Cache section
item? (Just as cookies?)

WebCore/loader/appcache/ApplicationCacheGroup.cpp:476
 +      // Because willSendRequest only gets called during redirects, we
initialize
Is this appcache loader's specifics?

WebCore/loader/appcache/ApplicationCacheGroup.cpp:493
 +         
page->inspectorController()->willSendRequest(m_currentResourceIdentifier,
request, redirectResponse);
Pardon my ignorance, but what exactly this loader is loading. Options are:
1) it fetches manifest file
2) it fetches and caches resources in manifest
3) it loads cached resources mentioned in manifest

WebCore/page/Page.cpp:787
 +  InspectorApplicationCacheAgent* Page::inspectorApplicationCacheAgent()
const
Given that the use of this guy is limited to the Page and AppCache class, I'd
inline access path in Page and get it via controller in AppCache subsystem. In
contrast, timeline has a convenience getter here because it is used all over
the place in WebCore

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