Bug 91831

Summary: Web Inspector: Add deleteEntry command and deletionCompleted event to FileSystemAgent.
Product: WebKit Reporter: Taiju Tsuiki <tzik>
Component: Web Inspector (Deprecated)Assignee: Taiju Tsuiki <tzik>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, dglazkov, joepeck, keishi, loislo, peter+ews, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 91861, 93933    
Bug Blocks: 68203, 91709, 93940    
Attachments:
Description Flags
Patch
none
Patch none

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.