Bug 50695

Summary: Web Inspector: Remove FileSystem support from the inspector.
Product: WebKit Reporter: Ilya Tikhonovsky <loislo>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Description Flags
[patch] initial version
yurys: review-
[PATCH] Proposed change.
yurys: review-
[PATCH] Review comments addressed.
yurys: review-
[PATCH] Review comments addressed.
[PATCH] Review comments addressed. yurys: review+

Description Ilya Tikhonovsky 2010-12-08 10:06:48 PST
As discussed with Pavel I'm removing FileSystem related code from inspector.
Comment 1 Ilya Tikhonovsky 2010-12-08 10:16:59 PST
Created attachment 75921 [details]
[patch] initial version
Comment 2 Yury Semikhatsky 2010-12-08 13:07:17 PST
Comment on attachment 75921 [details]
[patch] initial version

Guys, could you provide some context since even those sitting next to you in the same room don't know what are your the plans on this feature?
Comment 3 Pavel Feldman 2011-02-08 05:04:37 PST
It sounds like Filesystem API is going to be changed significantly and inspector support as is won't make any sense. Hence, let us nuke it until we know what exactly we want from it.
Comment 4 Pavel Feldman 2011-02-09 09:51:00 PST
Created attachment 81825 [details]
[PATCH] Proposed change.
Comment 5 Ilya Tikhonovsky 2011-02-09 10:45:38 PST
Comment on attachment 81825 [details]
[PATCH] Proposed change.

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

> Source/WebCore/inspector/InspectorApplicationCacheAgent.cpp:65
> -    DocumentLoader* documentLoader = m_inspectorAgent->inspectedPage()->mainFrame()->loader()->documentLoader();
> -    if (documentLoader) {
> -        ApplicationCacheHost* host = documentLoader->applicationCacheHost();
> +    if (m_documentLoader) {
> +        ApplicationCacheHost* host = m_documentLoader->applicationCacheHost();

What about the lifetime of documentLoader instance?

Others looks good to me.
Comment 6 Yury Semikhatsky 2011-02-10 08:48:04 PST
Comment on attachment 81825 [details]
[PATCH] Proposed change.

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

> Source/WebCore/inspector/InspectorApplicationCacheAgent.cpp:64
> +    if (m_documentLoader) {

Main frame may change on navigation and the loader will change as well. r- for this.
Comment 7 Pavel Feldman 2011-02-10 09:08:17 PST
Created attachment 81987 [details]
[PATCH] Review comments addressed.
Comment 8 Pavel Feldman 2011-02-10 09:08:39 PST
> What about the lifetime of documentLoader instance?

Good catch, thanks.
Comment 9 Yury Semikhatsky 2011-02-10 09:10:24 PST
Comment on attachment 81987 [details]
[PATCH] Review comments addressed.

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

> Source/WebCore/inspector/InspectorAgent.cpp:536
> +        m_applicationCacheAgent->didCommitLoad(loader);

You need to call this for main frame loader only.
Comment 10 Pavel Feldman 2011-02-10 09:11:07 PST
Created attachment 81988 [details]
[PATCH] Review comments addressed.
Comment 11 Pavel Feldman 2011-02-10 09:15:12 PST
Created attachment 81990 [details]
[PATCH] Review comments addressed.
Comment 12 Pavel Feldman 2011-02-10 09:44:05 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
        D       Source/WebCore/inspector/InspectorFileSystemAgent.cpp
        D       Source/WebCore/inspector/InspectorFileSystemAgent.h
        D       Source/WebCore/inspector/front-end/FileSystemView.js
        M       Source/WebCore/CMakeLists.txt
        M       Source/WebCore/ChangeLog
        M       Source/WebCore/GNUmakefile.am
        M       Source/WebCore/WebCore.gypi
        M       Source/WebCore/WebCore.pro
        M       Source/WebCore/WebCore.vcproj/WebCore.vcproj
        M       Source/WebCore/WebCore.xcodeproj/project.pbxproj
        M       Source/WebCore/inspector/Inspector.idl
        M       Source/WebCore/inspector/InspectorAgent.cpp
        M       Source/WebCore/inspector/InspectorAgent.h
        M       Source/WebCore/inspector/InspectorApplicationCacheAgent.cpp
        M       Source/WebCore/inspector/InspectorApplicationCacheAgent.h
        M       Source/WebCore/inspector/InspectorInstrumentation.cpp
        M       Source/WebCore/inspector/front-end/ResourcesPanel.js
        M       Source/WebCore/inspector/front-end/Settings.js
        M       Source/WebCore/inspector/front-end/WebKit.qrc
        M       Source/WebCore/inspector/front-end/inspector.html
Committed r78234