Bug 87852 - [Chromium] Call fileUtilities methods directly
: [Chromium] Call fileUtilities methods directly
Status: RESOLVED FIXED
: WebKit
WebKit Misc.
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 82948
  Show dependency treegraph
 
Reported: 2012-05-30 07:39 PST by
Modified: 2012-05-30 19:31 PST (History)


Attachments
Patch (11.87 KB, patch)
2012-05-30 07:40 PST, Mark Pilgrim (Google)
no flags Review Patch | Details | Formatted Diff | Diff
Patch (12.00 KB, patch)
2012-05-30 12:08 PST, Mark Pilgrim (Google)
no flags Review Patch | Details | Formatted Diff | Diff
Patch (11.89 KB, patch)
2012-05-30 13:24 PST, Mark Pilgrim (Google)
no flags Review Patch | Details | Formatted Diff | Diff
Patch (11.88 KB, patch)
2012-05-30 13:30 PST, Mark Pilgrim (Google)
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-05-30 07:39:46 PST
[Chromium] Call fileUtilities methods directly
------- Comment #1 From 2012-05-30 07:40:25 PST -------
Created an attachment (id=144812) [details]
Patch
------- Comment #2 From 2012-05-30 07:41:56 PST -------
Please check the changes to DragDataChromium.cpp very carefully! I'm not sure I got the casts right.
------- Comment #3 From 2012-05-30 09:40:05 PST -------
(From update of attachment 144812 [details])
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)
------- Comment #4 From 2012-05-30 12:08:36 PST -------
Created an attachment (id=144890) [details]
Patch
------- Comment #5 From 2012-05-30 12:09:06 PST -------
(From update of attachment 144890 [details])
Added intermediate variables, cast to String properly.
------- Comment #6 From 2012-05-30 13:21:55 PST -------
(From update of attachment 144890 [details])
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.
------- Comment #7 From 2012-05-30 13:24:38 PST -------
Created an attachment (id=144904) [details]
Patch
------- Comment #8 From 2012-05-30 13:25:05 PST -------
(From update of attachment 144904 [details])
Moved variable declaration.
------- Comment #9 From 2012-05-30 13:28:03 PST -------
(From update of attachment 144904 [details])
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.
------- Comment #10 From 2012-05-30 13:30:41 PST -------
Created an attachment (id=144907) [details]
Patch
------- Comment #11 From 2012-05-30 19:31:54 PST -------
(From update of attachment 144907 [details])
Clearing flags on attachment: 144907

Committed r119024: <http://trac.webkit.org/changeset/119024>
------- Comment #12 From 2012-05-30 19:31:59 PST -------
All reviewed patches have been landed.  Closing bug.