Bug 87852

Summary: [Chromium] Call fileUtilities methods directly
Product: WebKit Reporter: Mark Pilgrim (Google) <pilgrim>
Component: WebKit Misc.Assignee: Mark Pilgrim (Google) <pilgrim>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, fishd, haraken, jamesr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 82948    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Mark Pilgrim (Google)
Reported 2012-05-30 07:39:46 PDT
[Chromium] Call fileUtilities methods directly
Attachments
Patch (11.87 KB, patch)
2012-05-30 07:40 PDT, Mark Pilgrim (Google)
no flags
Patch (12.00 KB, patch)
2012-05-30 12:08 PDT, Mark Pilgrim (Google)
no flags
Patch (11.89 KB, patch)
2012-05-30 13:24 PDT, Mark Pilgrim (Google)
no flags
Patch (11.88 KB, patch)
2012-05-30 13:30 PDT, Mark Pilgrim (Google)
no flags
Mark Pilgrim (Google)
Comment 1 2012-05-30 07:40:25 PDT
Mark Pilgrim (Google)
Comment 2 2012-05-30 07:41:56 PDT
Please check the changes to DragDataChromium.cpp very carefully! I'm not sure I got the casts right.
James Robinson
Comment 3 2012-05-30 09:40:05 PDT
Comment on attachment 144812 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144812&action=review > Source/WebCore/platform/chromium/DragDataChromium.cpp:65 > + url = static_cast<KURL>(WebKit::Platform::current()->fileUtilities()->filePathToURL(static_cast<String>(WebKit::Platform::current()->fileUtilities()->getAbsolutePath(m_platformDragData->filenames()[0])))); This needs some temporaries at least - I can't tell if it's correct or not. static_cast<> is the wrong way to convert from, say, a WebString to a String - instead you want to call String(myWebString)
Mark Pilgrim (Google)
Comment 4 2012-05-30 12:08:36 PDT
Mark Pilgrim (Google)
Comment 5 2012-05-30 12:09:06 PDT
Comment on attachment 144890 [details] Patch Added intermediate variables, cast to String properly.
Adam Barth
Comment 6 2012-05-30 13:21:55 PDT
Comment on attachment 144890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144890&action=review > Source/WebCore/platform/chromium/DragDataChromium.cpp:62 > + String path; You can move this declaration inside the "else if" since it's only used there.
Mark Pilgrim (Google)
Comment 7 2012-05-30 13:24:38 PDT
Mark Pilgrim (Google)
Comment 8 2012-05-30 13:25:05 PDT
Comment on attachment 144904 [details] Patch Moved variable declaration.
Adam Barth
Comment 9 2012-05-30 13:28:03 PDT
Comment on attachment 144904 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144904&action=review > Source/WebCore/platform/chromium/DragDataChromium.cpp:66 > + String path; > + path = String(WebKit::Platform::current()->fileUtilities()->getAbsolutePath(m_platformDragData->filenames()[0])); These can just be put on one line.
Mark Pilgrim (Google)
Comment 10 2012-05-30 13:30:41 PDT
WebKit Review Bot
Comment 11 2012-05-30 19:31:54 PDT
Comment on attachment 144907 [details] Patch Clearing flags on attachment: 144907 Committed r119024: <http://trac.webkit.org/changeset/119024>
WebKit Review Bot
Comment 12 2012-05-30 19:31:59 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.