Bug 126369 - Web Inspector: Remove support for FileSystem in Frontend.
Summary: Web Inspector: Remove support for FileSystem in Frontend.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Seokju Kwon
URL:
Keywords: InRadar
Depends on:
Blocks: 126236
  Show dependency treegraph
 
Reported: 2014-01-01 18:13 PST by Seokju Kwon
Modified: 2014-01-06 16:48 PST (History)
9 users (show)

See Also:


Attachments
Patch (11.99 KB, patch)
2014-01-01 18:16 PST, Seokju Kwon
no flags Details | Formatted Diff | Diff
Patch (28.59 KB, patch)
2014-01-06 16:38 PST, Seokju Kwon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Seokju Kwon 2014-01-01 18:13:28 PST
Remove leftover codes from protocol after r156692.
Comment 1 Radar WebKit Bug Importer 2014-01-01 18:13:41 PST
<rdar://problem/15736396>
Comment 2 Seokju Kwon 2014-01-01 18:16:56 PST
Created attachment 220199 [details]
Patch
Comment 3 Joseph Pecoraro 2014-01-06 10:59:50 PST
Comment on attachment 220199 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=220199&action=review

r+ but there is more that you can delete! There are corresponding InspectorFrontendHost APIs which I think can be removed.

InspectorFrontendClient.h
78:    virtual bool supportsFileSystems() = 0;
79:    virtual void requestFileSystems() = 0;
80:    virtual void addFileSystem() = 0;
81:    virtual void removeFileSystem(const String& fileSystemPath) = 0;

InspectorFrontendClientLocal.h
79:    virtual bool supportsFileSystems() { return false; }
80:    virtual void requestFileSystems() { }
81:    virtual void addFileSystem() { }
82:    virtual void removeFileSystem(const String&) { }

InspectorFrontendHost.cpp
316:bool InspectorFrontendHost::supportsFileSystems()
319:        return m_client->supportsFileSystems();
323:void InspectorFrontendHost::requestFileSystems()
326:        m_client->requestFileSystems();
329:void InspectorFrontendHost::addFileSystem()
332:        m_client->addFileSystem();
335:void InspectorFrontendHost::removeFileSystem(const String& fileSystemPath)
338:        m_client->removeFileSystem(fileSystemPath);

InspectorFrontendHost.h
87:    bool supportsFileSystems();
88:    void requestFileSystems();
89:    void addFileSystem();
90:    void removeFileSystem(const String& fileSystemPath);

> Source/WebInspectorUI/UserInterface/InspectorWebBackendCommands.js:-162
> -// FileSystem.

I would suggest also deleting domain from Legacy protocols as well:

    {
        "domain": "FileSystem",
        ...
    },

For the reasons mentioned earlier in comments on other bugs (we don't plan to support it, and don't want naming conflicts if we add something in the future. We aren't keeping these pristine anyways).

    Source/WebInspectorUI/Versions/Inspector-iOS-6.0.json
    Source/WebInspectorUI/Versions/Inspector-iOS-7.0.json

Then rerun update-InspectorBackendCommands which will update UserInterface/Legacy/*/InspectorWebBackendCommands.js
Comment 4 Seokju Kwon 2014-01-06 16:17:08 PST
(In reply to comment #3)
> (From update of attachment 220199 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=220199&action=review
> 
> r+ but there is more that you can delete! There are corresponding InspectorFrontendHost APIs which I think can be removed.
> 
> InspectorFrontendClient.h
> 78:    virtual bool supportsFileSystems() = 0;
> 79:    virtual void requestFileSystems() = 0;
> 80:    virtual void addFileSystem() = 0;
> 81:    virtual void removeFileSystem(const String& fileSystemPath) = 0;
> 
> InspectorFrontendClientLocal.h
> 79:    virtual bool supportsFileSystems() { return false; }
> 80:    virtual void requestFileSystems() { }
> 81:    virtual void addFileSystem() { }
> 82:    virtual void removeFileSystem(const String&) { }
> 
> InspectorFrontendHost.cpp
> 316:bool InspectorFrontendHost::supportsFileSystems()
> 319:        return m_client->supportsFileSystems();
> 323:void InspectorFrontendHost::requestFileSystems()
> 326:        m_client->requestFileSystems();
> 329:void InspectorFrontendHost::addFileSystem()
> 332:        m_client->addFileSystem();
> 335:void InspectorFrontendHost::removeFileSystem(const String& fileSystemPath)
> 338:        m_client->removeFileSystem(fileSystemPath);
> 
> InspectorFrontendHost.h
> 87:    bool supportsFileSystems();
> 88:    void requestFileSystems();
> 89:    void addFileSystem();
> 90:    void removeFileSystem(const String& fileSystemPath);
> 
> > Source/WebInspectorUI/UserInterface/InspectorWebBackendCommands.js:-162
> > -// FileSystem.
> 
> I would suggest also deleting domain from Legacy protocols as well:
> 
>     {
>         "domain": "FileSystem",
>         ...
>     },
> 
> For the reasons mentioned earlier in comments on other bugs (we don't plan to support it, and don't want naming conflicts if we add something in the future. We aren't keeping these pristine anyways).
> 
>     Source/WebInspectorUI/Versions/Inspector-iOS-6.0.json
>     Source/WebInspectorUI/Versions/Inspector-iOS-7.0.json
> 
> Then rerun update-InspectorBackendCommands which will update UserInterface/Legacy/*/InspectorWebBackendCommands.js

Thanks, I'll clean them up as well.
Comment 5 Seokju Kwon 2014-01-06 16:38:20 PST
Created attachment 220469 [details]
Patch
Comment 6 WebKit Commit Bot 2014-01-06 16:48:26 PST
Comment on attachment 220469 [details]
Patch

Clearing flags on attachment: 220469

Committed r161385: <http://trac.webkit.org/changeset/161385>
Comment 7 WebKit Commit Bot 2014-01-06 16:48:29 PST
All reviewed patches have been landed.  Closing bug.