Bug 77881

Summary: [filesystem/Chromium] crackFileSystemURL needs to use innerURL
Product: WebKit Reporter: Eric U. <ericu>
Component: DOMAssignee: Eric U. <ericu>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, kinuko
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch abarth: review+

Description Eric U. 2012-02-06 09:57:35 PST
We need to update AsyncFileSystemChromium::crackFileSystemURL to use innerURL if it's present.
Once http://codereview.chromium.org/7811006 lands, it always will, and we can remove the old code.  Patch to follow shortly.
Comment 1 Eric U. 2012-02-06 11:22:50 PST
Created attachment 125675 [details]
Patch
Comment 2 Adam Barth 2012-02-06 13:10:51 PST
Comment on attachment 125675 [details]
Patch

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

> Source/WebKit/chromium/src/AsyncFileSystemChromium.cpp:67
> +        String typeString = url.innerURL()->path().substring(1);

There's probably a way to do this that saves some mallocs.  If this is performance-sensitive code, we might want to investigate further.
Comment 3 Adam Barth 2012-02-06 13:11:13 PST
Comment on attachment 125675 [details]
Patch

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

> Source/WebKit/chromium/src/AsyncFileSystemChromium.cpp:77
> +        filePath = decodeURLEscapeSequences(url.path());

Not the innerURLs path?
Comment 4 Eric U. 2012-02-06 13:14:05 PST
Comment on attachment 125675 [details]
Patch

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

>> Source/WebKit/chromium/src/AsyncFileSystemChromium.cpp:67
>> +        String typeString = url.innerURL()->path().substring(1);
> 
> There's probably a way to do this that saves some mallocs.  If this is performance-sensitive code, we might want to investigate further.

I don't expect it to be worth optimizing; certainly not right now.

>> Source/WebKit/chromium/src/AsyncFileSystemChromium.cpp:77
>> +        filePath = decodeURLEscapeSequences(url.path());
> 
> Not the innerURLs path?

Nope; the innerURL's path tells us which storage area ["/temporary" or "/persistent"], and the outer URL gets the virtual path.
Comment 5 Eric U. 2012-02-06 14:59:32 PST
Committed r106856: <http://trac.webkit.org/changeset/106856>