[Webkit-unassigned] [Bug 72456] [Inspector][FileSystem] Capture DOMFileSystem object

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Pavel Feldman <pfeldman at chromium.org>  2011-11-16 02:53:59 PST ---
(From update of attachment 115352)
View in context: https://bugs.webkit.org/attachment.cgi?id=115352&action=review

Looks good except for blank JavaScript files.

> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:73
> +    m_instrumentingAgents->setInspectorFileSystemAgent(this);

We don't like auxiliary agents getting into the instrumenting mode in the constructor. A much better pattern is to introduce ::enable and ::disable commands that would be called by the front-end lazily. No events should be sent to the front-end unless it is subscribed to them via the ::enable call. Your FS support on the front-end will then query FS state lazily upon Resources panel open via calling ::enable, ::getFileSystems and receive incremental updates from the instrumentation.

In case there is no place you could get active filesystems at a random time, you can start instrumenting in here, but please do not send notifications unless ::enable has been called.

> Source/WebCore/inspector/InspectorFileSystemInstrumentation.h:40
> +inline void InspectorInstrumentation::didOpenFileSystem(PassRefPtr<DOMFileSystem> fileSystem)

Given that you don't actually include any headers from this file, it can be a part of the InspectorInstrumentation. All we really care about is for InspectorInstrumentation not to get too many imports.

> Source/WebCore/inspector/InspectorInstrumentation.cpp:778
> +    if (!inspectorAgent || !inspectorAgent->enabled())

You need this check since you enter instrumenting mode prior to the inspector opening. You would not have needed it otherwise.

> Source/WebCore/inspector/front-end/FileSystem.js:1
> +/*

There is no need to add blank JavaScript files. Please add them with the content in a subsequent change. You will need to add them into the Source/WebCore/WebCore.vcproj/WebCore.vcproj as well.

> Source/WebCore/inspector/front-end/FileSystemView.js:1
> +/*

ditto

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