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

[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 #60486|review?                     |
               Flag|                            |




--- Comment #53 from Pavel Feldman <pfeldman at chromium.org>  2010-07-04 23:00:34 PST ---
(From update of attachment 60486)
WebCore/inspector/InspectorApplicationCacheAgent.cpp:72
 +      ApplicationCacheHost* host = m_inspectorController->inspectedPage()->mainFrame()->loader()->documentLoader()->applicationCacheHost();
How about you cache documentLoader in a var above?

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

WebCore/loader/appcache/ApplicationCacheHost.cpp:246
 +  void ApplicationCacheHost::fillResourceList(InspectorApplicationCacheAgent::ResourceInfoList* 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. I'd rather make inspector controller this hosts' friend and let it read the applicationCache by itself. 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.

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