Bug 172228 – A URL type is vended for a non-URL plain text string when starting data interaction

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Wenson Hsieh]

<rdar://problem/32166729>

[Wenson Hsieh]

Created attachment 310421 [details]
Patch

[Andy Estes]

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 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]

Created attachment 310433 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 310433 [details]
Patch for landing

Clearing flags on attachment: 310433

Committed r216997: <http://trac.webkit.org/changeset/216997>