[webkit-reviews] review granted: [Bug 72456] [Inspector][FileSystem] Capture DOMFileSystem object : [Attachment 116015] Patch (add a entry to CodeGeneratorInspector.py)

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has granted Taiju Tsuiki
<tzik at chromium.org>'s request for review:
Bug 72456: [Inspector][FileSystem] Capture DOMFileSystem object
https://bugs.webkit.org/show_bug.cgi?id=72456

Attachment 116015: Patch (add a entry to CodeGeneratorInspector.py)
https://bugs.webkit.org/attachment.cgi?id=116015&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=116015&action=review


> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:59
> +    // Store |fileSystem| and notify the frontend using |fileSystemAdded|
event.

We are not using | | notation in WebKit.

> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:98
> +    m_frontend = 0;

You should clear the enabled state here:
m_enabled = false;
m_state->setBoolean(FileSystemAgentState::fileSystemAgentEnabled, m_enabled);

disconnecting front-end should end up in disabling instrumentation.

> Source/WebCore/inspector/InspectorFileSystemAgent.h:53
> +    void invalidateFileSystem(PassRefPtr<DOMFileSystem>);

fileSystemInvalidated? If this is a signal from WebCore, it should be named as
event.