Summary: | (Meta) Web Inspector: Add FileSystem support | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Taiju Tsuiki <tzik> | ||||||||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Taiju Tsuiki <tzik> | ||||||||||||||||||
Status: | RESOLVED INVALID | ||||||||||||||||||||
Severity: | Normal | CC: | apavlov, burg, bweinstein, ericbidelman, janx, joepeck, keishi, kinuko, loislo, paulirish, pfeldman, pmuellr, rik, yurys | ||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||
OS: | All | ||||||||||||||||||||
Bug Depends on: | 72456, 72691, 73301, 87635, 87724, 87856, 88602, 89066, 89190, 89191, 89420, 89555, 89642, 89739, 89961, 90244, 90361, 90439, 90518, 90529, 90592, 91709, 91732, 91831, 93930, 93933, 93940, 94301, 94908, 97524, 98092 | ||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||
Attachments: |
|
Description
Taiju Tsuiki
2011-09-15 18:09:59 PDT
Created attachment 107652 [details]
Patch
A patch for early version of FileSystem support.
Comment on attachment 107652 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=107652&action=review Overall looks good. I added some high-level comments. > Source/WebCore/inspector/Inspector.json:832 > + "domain": "FileSystem", All methods and parameters should be documented. > Source/WebCore/inspector/Inspector.json:845 > + { "name": "fsName", "type": "string", "description": "" }, No abbreviations in WebKit. > Source/WebCore/inspector/Inspector.json:853 > + "name": "addFileSystem", We prefer fileSystemAdded notation. Also, you should not generate any messages unless FileSystem domain is enabled. You should add "enable" and "disable" commands. You could also include "getFileSystems" command that returns all filesystems for the late attach. > Source/WebCore/inspector/Inspector.json:861 > + "name": "onSuccess", There should be no generic "onSuccess"/"onError" events. > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:17 > + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY You should use Google license template. > Source/WebCore/inspector/InspectorFileSystemAgent.h:17 > + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY ditto > Source/WebCore/inspector/InspectorFileSystemAgent.h:57 > + class FileSystemTask; It looks like it can be defined in the anonymous namespace in the .cpp file. > Source/WebCore/inspector/InspectorFileSystemAgent.h:58 > + class ReadDirectoryTask; ditto > Source/WebCore/inspector/InspectorInstrumentation.h:37 > +#include "DOMFileSystem.h" InspectorInstrumentation should not add includes. You should add InspectorFileSystemInstrumentation.h (just like InspectorConsoleInstrumentation.h has) > Source/WebCore/inspector/front-end/FileSystem.js:43 > + FileSystemAgent.readDirectory(name, path, requestId); We are using single callback for both success and error paths, you could just check whether resulting message is an error. Can we get a new patch? So excited! :) This seems like it could make for a good example of a Web Inspector extension. Would certainly be a good use case for a Web Inspector extension. If you can't build this with the extension API, the extension API isn't done yet. (In reply to comment #4) > This seems like it could make for a good example of a Web Inspector extension. Would certainly be a good use case for a Web Inspector extension. If you can't build this with the extension API, the extension API isn't done yet. Not everything FS debugging needs is exposed via web facing APIs. Like mapping to the actual file system, etc. You can't say that extension API isn't done unless it exposes certain FS instrumentation specifics. I think it is totally fine to implement support for FS as a part of the core inspector since FS is a part of HTML. I complete first version of FileSystem support. It seems too big as single patch, so I'm splitting it into small ones: - Adding files and capture DOMFileSystem object, - Adding FileSystem item in storage tree on Resources panel, - Adding explorer style split-view as FileSystem view, - Adding directory tree as left part of split-view, - Adding text file content view as right part of split-view, - Adding binary file summary view. First of them is https://bugs.webkit.org/show_bug.cgi?id=72456, I'll post review request soon after. and here is the second part: [Inspector][FileSystem] Add FileSystem item to storage tree https://bugs.webkit.org/show_bug.cgi?id=72691 Created attachment 116451 [details]
Patch
Created attachment 116454 [details]
Screenshot
Created attachment 144292 [details]
Patch
Created attachment 144496 [details]
Patch
Created attachment 144827 [details]
Patch
Created attachment 148237 [details]
Patch
Created attachment 150388 [details]
Patch
This feature is no longer in WebKit. |