Bug 91831 - Web Inspector: Add deleteEntry command and deletionCompleted event to FileSystemAgent.
Summary: Web Inspector: Add deleteEntry command and deletionCompleted event to FileSys...
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: Taiju Tsuiki
URL:
Keywords:
Depends on: 91861 93933
Blocks: 68203 91709 93940
  Show dependency treegraph
 
Reported: 2012-07-20 01:16 PDT by Taiju Tsuiki
Modified: 2012-08-22 00:50 PDT (History)
13 users (show)

See Also:


Attachments
Patch (12.81 KB, patch)
2012-08-14 00:36 PDT, Taiju Tsuiki
no flags Details | Formatted Diff | Diff
Patch (16.50 KB, patch)
2012-08-17 00:33 PDT, Taiju Tsuiki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Taiju Tsuiki 2012-07-20 01:16:13 PDT
Inspector should allow user to delete files and directories on FileSystem.
Comment 1 Taiju Tsuiki 2012-08-14 00:36:18 PDT
Created attachment 158245 [details]
Patch
Comment 2 Vsevolod Vlasov 2012-08-16 01:10:26 PDT
Comment on attachment 158245 [details]
Patch

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

> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:782
> +    ASSERT(m_frontendProvider->frontend());

Nit: consider extracting the following code

if (!m_enabled || !m_frontendProvider) {
    *error = "FileSystem agent is not enabled";
    return;
}
ASSERT(m_frontendProvider->frontend());

So that each protocol command started with 

if (!assertFrontend(error))
    return;

This way it should be shorter.
Comment 3 Taiju Tsuiki 2012-08-17 00:33:31 PDT
Created attachment 159027 [details]
Patch
Comment 4 Taiju Tsuiki 2012-08-17 00:34:50 PDT
Comment on attachment 158245 [details]
Patch

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

>> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:782
>> +    ASSERT(m_frontendProvider->frontend());
> 
> Nit: consider extracting the following code
> 
> if (!m_enabled || !m_frontendProvider) {
>     *error = "FileSystem agent is not enabled";
>     return;
> }
> ASSERT(m_frontendProvider->frontend());
> 
> So that each protocol command started with 
> 
> if (!assertFrontend(error))
>     return;
> 
> This way it should be shorter.

Looks nice. Done.
Comment 5 WebKit Review Bot 2012-08-17 01:03:46 PDT
Comment on attachment 159027 [details]
Patch

Attachment 159027 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13514708
Comment 6 Peter Beverloo (cr-android ews) 2012-08-17 01:09:12 PDT
Comment on attachment 159027 [details]
Patch

Attachment 159027 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13521286
Comment 7 Vsevolod Vlasov 2012-08-20 01:58:50 PDT
Comment on attachment 159027 [details]
Patch

Please fix "ReportErrorTask" building error.
Comment 8 Vsevolod Vlasov 2012-08-20 01:59:39 PDT
Comment on attachment 159027 [details]
Patch

Oh, I see you are adding it in another patch.
Comment 9 WebKit Review Bot 2012-08-22 00:50:16 PDT
Comment on attachment 159027 [details]
Patch

Clearing flags on attachment: 159027

Committed r126273: <http://trac.webkit.org/changeset/126273>
Comment 10 WebKit Review Bot 2012-08-22 00:50:21 PDT
All reviewed patches have been landed.  Closing bug.