Bug 50695 - Web Inspector: Remove FileSystem support from the inspector.
Summary: Web Inspector: Remove FileSystem support from the inspector.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-08 10:06 PST by Ilya Tikhonovsky
Modified: 2011-02-10 09:44 PST (History)
10 users (show)

See Also:


Attachments
[patch] initial version (41.01 KB, patch)
2010-12-08 10:16 PST, Ilya Tikhonovsky
yurys: review-
Details | Formatted Diff | Diff
[PATCH] Proposed change. (49.95 KB, patch)
2011-02-09 09:51 PST, Pavel Feldman
yurys: review-
Details | Formatted Diff | Diff
[PATCH] Review comments addressed. (50.45 KB, patch)
2011-02-10 09:08 PST, Pavel Feldman
yurys: review-
Details | Formatted Diff | Diff
[PATCH] Review comments addressed. (50.74 KB, patch)
2011-02-10 09:11 PST, Pavel Feldman
no flags Details | Formatted Diff | Diff
[PATCH] Review comments addressed. (50.79 KB, patch)
2011-02-10 09:15 PST, Pavel Feldman
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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