[webkit-reviews] review requested: [Bug 24529] WebInspector: HTML5 Offline Web Applications Support (ApplicationCache) : [Attachment 60500] [PATCH] Part 2: Pulling ApplicationCache Resources to Display in the Inspector.

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has asked	for review:
Bug 24529: WebInspector: HTML5 Offline Web Applications Support
(ApplicationCache)
https://bugs.webkit.org/show_bug.cgi?id=24529

Attachment 60500: [PATCH] Part 2: Pulling ApplicationCache Resources to Display
in the Inspector.
https://bugs.webkit.org/attachment.cgi?id=60500&action=review

------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
> WebCore/inspector/InspectorApplicationCacheAgent.cpp:72
>  +	  ApplicationCacheHost* host =
m_inspectorController->inspectedPage()->mainFrame()->loader()->documentLoader()
->applicationCacheHost();
> How about you cache documentLoader in a var above?

Done.


> WebCore/inspector/InspectorApplicationCacheAgent.cpp:117
>  +	      types.append("Master ");
> Looks like localizable names to me. Todo?

Hmm, yes I'll add a follow up patch to see if this is desired. Good catch.


> WebCore/loader/appcache/ApplicationCacheHost.cpp:246
>  +  void
ApplicationCacheHost::fillResourceList(InspectorApplicationCacheAgent::Resource
InfoList* resources)
> This I don't like at all. I believe inspector controller functionality should
be encapsulated.
> We should not force instrumented code to be too involved with the inspector
controller.
> [...] Or even better, add public applicationCacheForInspector method that
would call
> applicationCache(). And use it from the agent. In a sense, that's what you
are doing in this method.

Done.


> Btw, put the Google copyright in case you are using her code

Done.

Added Kativa to the ChangeLog and added copyright to
InspectorApplicationCacheAgent.cpp
for the implementation of the buildObject/Array methods, and other functions.
Updated
copyright in DOMAgent.js. All other files had minor changes or were up to date.