Bug 172228

Summary: A URL type is vended for a non-URL plain text string when starting data interaction
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: WebCore Misc.Assignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, commit-queue, megan_gardner, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
aestes: review+
Patch for landing none

Wenson Hsieh
Reported 2017-05-17 11:16:33 PDT
Attachments
Patch (8.39 KB, patch)
2017-05-17 12:00 PDT, Wenson Hsieh
aestes: review+
Patch for landing (8.56 KB, patch)
2017-05-17 13:15 PDT, Wenson Hsieh
no flags
Wenson Hsieh
Comment 1 2017-05-17 12:00:10 PDT
Andy Estes
Comment 2 2017-05-17 12:53:37 PDT
Comment on attachment 310421 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310421&action=review > Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:203 > +static void addRepresentationsForPlainText(RetainPtr<WebItemProviderRegistrationInfoList> itemsToRegister, const String& plainText) Passing a RetainPtr by value causes retain count churn. Why not just pass a WebItemProviderRegistrationInfoList *? > Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:209 > + NSURL *platformURL = [NSURL URLWithString:plainText]; > + if (URL(platformURL).isValid()) Is this materially different than constructing a URL with plainText as the relative URL and an empty base URL? Just wondering if we can avoid creating a transient NSURL. > Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:311 > + addRepresentationsForPlainText(itemsToRegister, textAsNSString); Can you pass text as the second argument instead of textAsNSString to avoid an unnecessary NSString-to-WTFString conversion?
Wenson Hsieh
Comment 3 2017-05-17 13:03:56 PDT
Comment on attachment 310421 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310421&action=review >> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:203 >> +static void addRepresentationsForPlainText(RetainPtr<WebItemProviderRegistrationInfoList> itemsToRegister, const String& plainText) > > Passing a RetainPtr by value causes retain count churn. Why not just pass a WebItemProviderRegistrationInfoList *? Good point, the RetainPtr isn't doing much here -- fixed. >> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:209 >> + if (URL(platformURL).isValid()) > > Is this materially different than constructing a URL with plainText as the relative URL and an empty base URL? Just wondering if we can avoid creating a transient NSURL. We'll have to create an NSURL anyways, since we want to use the platform way of writing to the UIItemProvider (which takes an NSURL *). The transient entity here is actually the URL, which I'm creating so that we can gate the "URL"-ness of the plain text we're registering on what WebCore identifies as a valid URL rather than what is needed to create an NSURL. >> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:311 >> + addRepresentationsForPlainText(itemsToRegister, textAsNSString); > > Can you pass text as the second argument instead of textAsNSString to avoid an unnecessary NSString-to-WTFString conversion? Oh, good point -- removed textAsNSString altogether.
Wenson Hsieh
Comment 4 2017-05-17 13:15:54 PDT
Created attachment 310433 [details] Patch for landing
WebKit Commit Bot
Comment 5 2017-05-17 13:56:08 PDT
Comment on attachment 310433 [details] Patch for landing Clearing flags on attachment: 310433 Committed r216997: <http://trac.webkit.org/changeset/216997>
Note You need to log in before you can comment on or make changes to this bug.