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

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Taiju Tsuiki <tzik at chromium.org>  2011-11-17 01:09:15 PST ---
(In reply to comment #9)
Thank you for reviewing.

> (From update of attachment 115352 [details])
> 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.
> 

Done. I had splitted out it, and now merged some of it back.

> 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.
> 

I see. In my previous CL, I added #include "DOMFileSystem.h" to InspectorInstrumentation.h.
Now, I moved it to InspectorFileSystemInstrumentation.h.

> > 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

Done.

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