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>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
[patch] initial version	yurys: review-
[PATCH] Proposed change.	yurys: review-
[PATCH] Review comments addressed.	yurys: review-
[PATCH] Review comments addressed.	none
[PATCH] Review comments addressed.	yurys: review+

Ilya Tikhonovsky

Reported 2010-12-08 10:06:48 PST

As discussed with Pavel I'm removing FileSystem related code from inspector.

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
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Ilya Tikhonovsky

Comment 1 2010-12-08 10:16:59 PST

Created attachment 75921 [details] [patch] initial version

Yury Semikhatsky

Comment 2 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?

Pavel Feldman

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

Pavel Feldman

Comment 4 2011-02-09 09:51:00 PST

Created attachment 81825 [details] [PATCH] Proposed change.

Ilya Tikhonovsky

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

Yury Semikhatsky

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

Pavel Feldman

Comment 7 2011-02-10 09:08:17 PST

Created attachment 81987 [details] [PATCH] Review comments addressed.

Pavel Feldman

Comment 8 2011-02-10 09:08:39 PST

> > What about the lifetime of documentLoader instance? > Good catch, thanks.

Yury Semikhatsky

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

Pavel Feldman

Comment 10 2011-02-10 09:11:07 PST

Created attachment 81988 [details] [PATCH] Review comments addressed.

Pavel Feldman

Comment 11 2011-02-10 09:15:12 PST

Created attachment 81990 [details] [PATCH] Review comments addressed.

Pavel Feldman

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

Note You need to log in before you can comment on or make changes to this bug.