Bug 171100 – Implement the Web Share API

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Rick Byers]

Web Share is a proposal to add an API to the web for sharing text, URLs and images to an arbitrary destination of the user's choice:
navigator.share({title: 'Example Page', url: 'https://example.com'});

Explainer: https://github.com/WICG/web-share/blob/master/docs/explainer.md
Draft interface: https://github.com/WICG/web-share/blob/master/docs/interface.md
Issues for questions, feedback, etc.: https://github.com/WICG/web-share/issues

This has been available behind an origin trial in Chrome since 55: https://www.chromestatus.com/feature/5668769141620736
I believe there is interest in shipping sometime soon in Chrome, but we'd love to get feedback from other potential implementors before we're locked into the API.

[Rick Byers]

Also relevant blog entry: https://developers.google.com/web/updates/2016/10/navigator-share

[Dean Jackson]

This is interesting. I'll add it to our status page.

[Radar WebKit Bug Importer]

<rdar://problem/31751734>

[Olivia Barnett]

Created attachment 347525 [details]
Patch

[Tim Horton]

Large things:

1) Needs a test. Or probably a few.
2) Needs a RuntimeEnabledFeature switch. Or some other kind of switch?
3) Needs to be rebased on trunk, the patch doesn't apply.
4) Probably requires an email to webkit-dev noting that you're implementing this (see https://webkit.org/feature-policy/).
5) Need to update the feature status JSON file (Source/WebCore/features.json IIRC)

[Simon Fraser (smfr)]

Comment on attachment 347525 [details]
Patch

Please include an update to Source/WebCore/features.json

[Tim Horton]

Comment on attachment 347525 [details]
Patch

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

> Source/WebKit/UIProcess/WebShareSheetResultListenerProxy.cp:2
> + * Copyright (C) 2010 Apple Inc. All rights reserved.

Get rid of this file. Also, it's 2018

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.h:52
> +- (void)presentWithParameters:(NSArray*)parameters resultListener:(WebKit::WebShareSheetResultListenerProxy*)listener completionHandler:(WTF::CompletionHandler<void(bool)>&&)completionHandler;

ObjC stars go on the right (NSArray *). The C++ one is right.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:94
> +- (void)presentWithParameters:(NSArray*)parameters resultListener:(WebKit::WebShareSheetResultListenerProxy*)listener completionHandler:(WTF::CompletionHandler<void(bool)>&&)completionHandler

You don't need WebShareSheetResultListener. Get rid of it everywhere. You just use the completion block mechanism instead, and don't have any API.

> Source/WebKit/WebProcess/WebPage/WebPage.h:1576
> +    RefPtr<WebShareSheetResultListener> m_activeShareSheetResultListener;

You don't need WebShareSheetResultListener

> Source/WebKit/WebProcess/WebPage/WebPage.h:1740
> +    HashMap<uint64_t, WTF::Function<void(bool granted)>> m_shareSheetResponseCallbackMap;

I don't think "granted" is the right word.

[Tim Horton]

Comment on attachment 347525 [details]
Patch

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

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:3
> + *

Please clean up the '#pragma mark's and FileUpload related things in this file.

[Wenson Hsieh]

Comment on attachment 347525 [details]
Patch

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

Overall, this looks great! A few minor comments:

> Source/WebCore/loader/EmptyClients.cpp:440
> +void EmptyChromeClient::runShareSheet(Frame&, ShareDataWithParsedURL&, WTF::CompletionHandler<void(bool)>&&)

Nit - the WTF:: here too.

> Source/WebCore/loader/EmptyClients.h:140
> +    void runShareSheet(Frame&, ShareDataWithParsedURL&, WTF::CompletionHandler<void(bool)>&&) final;

Nit - you shouldn't need an explicit WTF:: here.

> Source/WebKit/UIProcess/WebShareSheetResultListenerProxy.cp:49
> +void WebOpenPanelResultListenerProxy::chooseFiles(const Vector<WTF::String>& filenames, const String& displayString, const API::Data* iconImageData)

Nit - WTF:: here too.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4647
> +    NSMutableArray *shareDataArray = [[NSMutableArray alloc] init];

Nit - it looks like this leaks? You could consider changing this to:

auto shareDataArray = adoptNS([[NSMutableArray alloc] init]);

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4670
> +    ASSERT(_shareSheet.get() == shareSheet);

Nit - I don't think the .get() is necessary here.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:58
> +}

Nit - extra {}

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:68
> +    WKContentView *_view;

You could consider making this a WeakObjCPtr if WKShareSheet can outlive its WKContentView.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:72
> +#pragma clang diagnostic push

I don't think you need these 3 #pragma's

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:85
> +- (void)dealloc

Nit - I don't think you need to override -dealloc here.

> Source/WebKit/WebProcess/WebPage/WebPage.h:175
> +struct ShareData;

Hm...you shouldn't need to forward declare this if you're already including <WebCore/ShareData.h> above.

> Source/WebKit/WebProcess/WebPage/WebShareSheetResultListener.h:44
> +    void disconnectFromPage() { m_page = 0; }

m_page = nullptr;

If WebPage always outlives its WebShareSheetResultListener, I would suggest using a reference; otherwise, if this can potentially outlive its WebPage, you might want to consider holding a WeakPtr.

> Source/WebKitLegacy/ios/WebCoreSupport/WebChromeClientIOS.mm:160
> +void WebChromeClientIOS::runShareSheet(Frame&, ShareDataWithParsedURL&,  CompletionHandler<void(bool)>&&)

Nit - extra space before CompletionHandler

[Tim Horton]

I think you need to add a line to messages.py’s headers_for_type()’s special_cases pointing the message generator to ShareData.h for ShareDataWithParsedURL. Not sure how you got away without that locally!

[Tim Horton]

Comment on attachment 347525 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4644
> +    _shareSheet = adoptNS([[WKShareSheet alloc] initWithView:self]);

I think most of this logic should be inside WKShareSheet; hand ShareDataWithParsedURL all the way in there and do the conversion to ObjC types there.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4648
> +    if ([data.shareData.title length] > 0)

This is weird, because you’re going from WTFString to NSString just to check the length. WTFString has .isEmpty(), probably better to just use that.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4654
> +    if (data.url.isNull())

I think you meant to have a ! here :P

[Tim Horton]

Comment on attachment 347525 [details]
Patch

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

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:101
> +    auto completionHandlerBlock = BlockPtr<void((NSString *activityType, BOOL completed, NSArray *returnedItems, NSError *activityError))>::fromCallable([completionHandler = WTFMove(completionHandler), shareSheet = self](NSString *activityType, BOOL completed, NSArray *returnedItems, NSError *activityError) mutable {

You don’t need to name all the parameters in the BlockPtr type definition (I’m pretty sure).

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:108
> +    [self _presentFullscreenViewController:_shareSheetViewController.get() animated:NO];

We definitely want an animated presentation.

[Olivia Barnett]

Created attachment 347690 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 347690 [details]
Patch

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

Not a full review.

> Source/WebCore/page/Navigator.cpp:102
> +    if (m_frame->page()) {

Use an early return to avoid indenting all the rest of the code.

> Source/WebCore/page/Navigator.cpp:110
> +        if (!url.isValid())
> +            promise->reject(TypeError);

You should return here, right?

> Source/WebCore/page/Navigator.cpp:123
> +            return;

No need for the return.

> Source/WebKit/Shared/WebPreferences.yaml:763
> +  humanReadableName: "Web share"

Can someone read this and figure out what the feature is about?

> Source/WebKit/Shared/WebPreferences.yaml:764
> +  humanReadableDescription: "Web share API"

Web Share API?

[Tim Horton]

Needs to be rebased against master again

[Tim Horton]

Comment on attachment 347690 [details]
Patch

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

> Source/WebCore/page/Chrome.h:61
> +struct ShareDataWithParsedURL;

This should be sorted to the right place (alphabetically).

> Source/WebCore/page/ChromeClient.h:106
> +struct ShareDataWithParsedURL;

Sorting.

> Source/WebCore/page/Navigator.cpp:117
> +        m_frame->page()->chrome().runShareSheet(*m_frame, shareData, [documentReference = makeWeakPtr(m_frame->document()), promise = WTFMove(promise)] (bool completed) mutable {

Why do you capture the document? Also what's the 'mutable' about? (there might be a good reason?)

> Source/WebCore/page/Navigator.cpp:122
> +            promise->reject(Exception { AbortError, "Aborted by AbortSignal."_s });

Where did the weird string "Aborted by AbortSignal" come from?

> Source/WebKit/Scripts/webkit/messages.py:385
> +        'WebCore::ShareData': ['<WebCore/ShareData.h>'],

This one is not needed, this is just for overrides where the left and right hand side don't match.

> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:500
> +// Defaults to false

Is this comment a lie? (it is)

> Source/WebKit/UIProcess/WebPageProxy.h:185
> +struct ShareData;

sooooooooooooooort

> Source/WebKit/UIProcess/WebPageProxy.h:1481
> +    void runShareSheet(uint64_t frameID, const WebCore::ShareDataWithParsedURL&,  uint64_t contextID);

You've got one extra space before the uint64_t

Also I think we should rename this to callbackID everywhere (it's the ... ID... of the... callback. it doesn't provide any context). Ideally it would also have a typedef instead of being a uint64_t everywhere.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:66
> +#import <WebCore/ShareData.h>

Sort (should go with other WebCore includes). Not sure how the stylebot didn't complain about this one, since this is totally something it knows about (perhaps unlike the others)

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.h:28
> +#import <UIKit/UIViewController.h>

It's slightly unusual to import a single UIKit header, IMO, but maybe it's OK. <UIKit/UIKit.h> is a safe bet, though.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.h:37
> +class TextStream;

Why?

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.h:50
> +@property (nonatomic, assign) id <WKShareSheetDelegate> delegate;

I would put properties after all the methods. Especially it feels weird if init isn't first. Also I think I'd separate these differently:

-init

-present
-dismiss

@properties

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:76
> +        [shareDataArray addObject:(NSString*)data.shareData.title];

Lots of ObjC stars on the wrong side (should be (NSString *)) throughout this function.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:107
> +    [[UIViewController _viewControllerForFullScreenPresentationFromView:_view.get().get()] dismissViewControllerAnimated:YES completion:nil];

.getAutoreleased() returns an autoreleased raw pointer, so you can avoid this weird .get().get() (the first one returns a very-shortlived-and-unnecessary RetainPtr).

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:127
> +    [self _dismissDisplayAnimated:NO];

How do we ever get here with an existing presented VC?

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:129
> +    _presentationViewController = [UIViewController _viewControllerForFullScreenPresentationFromView:_view.get().get()];

Ditto.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3694
> +    

A few places you're adding unnecessary whitespace. I think maybe from when you had the listener.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6043
> +static uint64_t nextShareSheetContextId()

s/context/callback, like I said above

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6094
> +    ASSERT(callback);

I don't think there's any need for this assert, we're just about to crash if it's null, so there's no point crashing one line earlier only in debug builds :) No idea why the person above did that.

> Source/WebKit/WebProcess/WebPage/WebPage.h:1076
> +    void runShareSheet(WebCore::Frame&, uint64_t frameID, WebCore::ShareDataWithParsedURL&, WTF::CompletionHandler<void(bool)>&& callback);

Why does this take a WebCore::Frame? it doesn't look like you need it for anything.

ACTUALLY if I follow it through, you don't need the frameID for anything either, right? I think these are vestiges of the code you copied.  This is the fun part where after growing for a while, your patch collapses to be smaller :)

> Source/WebKit/WebProcess/WebPage/WebPage.h:1735
> +    void runShareSheetResponse(bool wasCompleted, uint64_t contextId);

This is definitely not the right place for a function. WebKit style puts all member variables below all member functions.

> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:3016
> +- (BOOL)webShareEnabled

I don't think there's any need for WebKitLegacy SPI to turn this on/off. I don't think we'll likely implement for WebKitLegacy.

[Tim Horton]

Comment on attachment 347690 [details]
Patch

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

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:75
> +    if (!data.shareData.title.isEmpty())

I think it's probably wrong to put the title directly in the share items. Looking at the spec, it seems like 'text' and 'url' are the things you're sharing, and title is sort of metadata *about* the thing you're sharing. So I think we want to be a little smarter here. We have some options:

1. We could just drop the title on the floor. The spec says this is OK.
2. We could use the weird _title SPI on NSURL to smush the title into the URL, if we have one. Not sure any clients will care, but we could consider smartening some of them up.
3. If you only have a title, and no text nor URL-to-smush-into, you could use the title as the text.

I think a combination of 2 and 3 is probably the "smartest" for now, but would also be OK with 1 for the first go-around. But what we have right now is probably not workable (mostly because the share sheet generally has to pick *one* of the items to share, and it would be a shame if it shared the title instead of the more-useful URL or text).

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:82
> +    UIActivityViewController *shareSheetController = [[UIActivityViewController alloc] initWithActivityItems:shareDataArray.get() applicationActivities:nil];

Please put the adoptNS here, sink it into a local RetainPtr, and then assign to the instance variable later. Otherwise this gets everyone's leak spidey-sense tingling.

[Olivia Barnett]

Created attachment 347979 [details]
Patch

[EWS Watchlist]

Attachment 347979 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebKit/ChangeLog:12:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 3 in 48 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Comment on attachment 347979 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        No new tests (OOPS!).

Except for "No new tests", this is quite close to being ready!

> Source/WebCore/page/Navigator.cpp:108
> +        

unnecessary newline

> Source/WebCore/page/Navigator.cpp:130
> +        promise->reject(Exception { AbortError, "Share sheet cancelled, rejecting promise with abort."_s });

I don't think we should use the word "share sheet" in shared code, that's kind of an iOSism. Maybe just remove the word "sheet"? I'm not sure. Also not totally sure you need to describe that you're rejecting the promise in the promise rejection, but follow whatever you see other people doing.

> Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:575
> +bool PageClientImpl::handleRunShareSheet(WebPageProxy*, const ShareDataWithParsedURL& shareData, WTF::CompletionHandler<void(bool)>&& completionHandler)

Let's just call it showShareSheet here too!

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.h:37
> +namespace WTF {
> +}

??

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.h:46
> +namespace API {
> +}
> +
> +namespace WebKit {
> +}

?????

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:84
> +            url._title = data.shareData.title;

You'll need to import UIKitSPI.h (hopefully it already knows about _title).

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:93
> +    NSURL *urlWithTitle = (NSURL *)data.url;
> +    urlWithTitle._title = data.shareData.title;

I think this is not still real. urlWithTitle goes nowhere, and the real code for this is above in the !data.url.isNull() block

> Source/WebKitLegacy/mac/WebView/WebPreferenceKeysPrivate.h:176
> +#define WebKitWebShareEnabledPreferenceKey @"WebKitWebShareEnabled"

You don't need this.

[Olivia Barnett]

Created attachment 348022 [details]
Patch

[EWS Watchlist]

Attachment 348022 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 2 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Comment on attachment 348022 [details]
Patch

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

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.h:45
> +@property (nonatomic, assign) id <WKShareSheetDelegate> delegate;

This should be weak.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:35
> +#import "UIKitSPI.h"

LOL, _title is only in  the WebCore UIKitSPI.h, not the one you included here. So sorry :( I think you'll need to add _title to the WebKit one. (you can grab those five lines from the WebCore one).

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:74
> +    bool arrayContainsElements = false;

Why this instead of just testing [shareDataArray count]? (Either way is fine I guess)

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:137
> +    // May need to clear/dismiss VC?

Get rid of this comment.

[Olivia Barnett]

Created attachment 348033 [details]
Patch

[EWS Watchlist]

Attachment 348033 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebKit/ChangeLog:9:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 3 in 48 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 348033 [details]
Patch

Attachment 348033 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8974635

New failing tests:
fast/dom/navigator-detached-no-crash.html
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
imported/w3c/web-platform-tests/web-share/share-securecontext.http.html
imported/w3c/web-platform-tests/web-share/share-without-user-gesture.https.html
imported/w3c/web-platform-tests/web-share/idlharness.https.html
imported/w3c/web-platform-tests/web-share/share-empty.https.html

[EWS Watchlist]

Created attachment 348037 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Tim Horton]

You'll want to rebaseline the WPTs that became passes, and probably fix things to make the ones that became fails not fails.

[Olivia Barnett]

Created attachment 348062 [details]
Patch

[EWS Watchlist]

Attachment 348062 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 2 in 56 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Comment on attachment 348062 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        No new tests (OOPS!).

You can totally change this to "No new tests; adjusted expectations for existing tests." and get rid of the OOPS and be happy.

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:433
> +@property (nonatomic, retain, setter=_setContainerViews:)
> +NSArray *_containerViews;

What happened here?

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:34
> +#import "APIArray.h"
> +#import "APIData.h"
> +#import "APIOpenPanelParameters.h"
> +#import "APIString.h"

I doubt you need these.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:40
> +#import "WKData.h"
> +#import "WKStringCF.h"
> +#import "WKURLCF.h"
> +#import "WebIconUtilities.h"

I doubt you need these.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:42
> +#import <MobileCoreServices/MobileCoreServices.h>

I doubt you need this.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:43
> +#import <WebCore/LocalizedStrings.h>

I doubt you need this.

> LayoutTests/imported/w3c/web-platform-tests/web-share/idlharness.https-expected.txt:3
> -FAIL Navigator interface: operation share(ShareData) assert_own_property: interface prototype object missing non-static operation expected property "share" missing
> +FAIL Navigator interface: operation share(ShareData) assert_equals: property has wrong .length expected 0 but got 1

Need to ask Sam about this.

[EWS Watchlist]

Comment on attachment 348062 [details]
Patch

Attachment 348062 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8978254

New failing tests:
imported/w3c/web-platform-tests/web-share/share-without-user-gesture.https.html
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
imported/w3c/web-platform-tests/web-share/idlharness.https.html
imported/w3c/web-platform-tests/web-share/share-empty.https.html

[EWS Watchlist]

Created attachment 348069 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348062 [details]
Patch

Attachment 348062 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8978290

New failing tests:
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
fast/dom/navigator-detached-no-crash.html
imported/w3c/web-platform-tests/web-share/share-securecontext.http.html

[EWS Watchlist]

Created attachment 348071 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348062 [details]
Patch

Attachment 348062 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8978319

New failing tests:
imported/w3c/web-platform-tests/web-share/share-without-user-gesture.https.html
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
imported/w3c/web-platform-tests/web-share/idlharness.https.html
imported/w3c/web-platform-tests/web-share/share-empty.https.html

[EWS Watchlist]

Created attachment 348074 [details]
Archive of layout-test-results from ews113 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Olivia Barnett]

Created attachment 348153 [details]
Patch

[EWS Watchlist]

Attachment 348153 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 56 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 348153 [details]
Patch

Attachment 348153 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8996788

New failing tests:
imported/w3c/web-platform-tests/web-share/share-without-user-gesture.https.html
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
imported/w3c/web-platform-tests/web-share/idlharness.https.html
imported/w3c/web-platform-tests/web-share/share-empty.https.html

[EWS Watchlist]

Created attachment 348163 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348153 [details]
Patch

Attachment 348153 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8997025

New failing tests:
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
fast/dom/navigator-detached-no-crash.html
imported/w3c/web-platform-tests/web-share/share-securecontext.http.html

[EWS Watchlist]

Created attachment 348171 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348153 [details]
Patch

Attachment 348153 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8996925

New failing tests:
imported/w3c/web-platform-tests/web-share/share-without-user-gesture.https.html
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
imported/w3c/web-platform-tests/web-share/idlharness.https.html
imported/w3c/web-platform-tests/web-share/share-empty.https.html

[EWS Watchlist]

Created attachment 348178 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Olivia Barnett]

Created attachment 348225 [details]
Patch

[EWS Watchlist]

Attachment 348225 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 65 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Wenson Hsieh]

Comment on attachment 348225 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        No new tests; adjusted expectations for existing tests.

You should mention your new test in fast/events/ios!

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:384
> +- (void)invokeShareSheetWithResolution:(BOOL)resolved WK_API_AVAILABLE(ios(11.3));

This should be WK_API_AVAILABLE(ios(WK_IOS_TBA));

> Source/WebKit/UIProcess/PageClient.h:205
> +    virtual bool showShareSheet(WebPageProxy*, const WebCore::ShareDataWithParsedURL&,  WTF::CompletionHandler<void (bool)>&&) { return false; }

Nit - extra space before WTF::

> Source/WebKit/UIProcess/WebPageProxy.cpp:5254
> +    

Nit - it doesn't look like this extra whitespace change is intended

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.h:46
> +@property (nonatomic, weak, assign) id <WKShareSheetDelegate> delegate;

Nit - I don't think you need the assign here.

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:40
> +@interface _WKShareSheetItem : NSObject

Hm...what's _WKShareSheetItem used for?

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:80
> +    RetainPtr<UIActivityViewController> shareSheetController = adoptNS([[UIActivityViewController alloc] initWithActivityItems:shareDataArray.get() applicationActivities:nil]);

Nit - s/RetainPtr<UIActivityViewController>/auto/

> Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:88
> +    _shareSheetViewController = shareSheetController;

This can be _shareSheetViewController = WTFMove(shareSheetController);

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6085
> +void WebPage::runShareSheetResponse(bool wasGranted, uint64_t callbackID)

s/uint64_t/ShareSheetCallbackID/ here?

> Source/WebKit/WebProcess/WebPage/WebPage.h:1734
> +    HashMap<uint64_t, WTF::Function<void(bool completed)>> m_shareSheetResponseCallbackMap;

ShareSheetCallbackID here too?

> LayoutTests/fast/events/ios/share.html:20
> +                    navigator.share({ title: "Example Page", url: "url", text: "text"}).then((result) => {

Nit - Indentation is a bit off here too.

Also, a space between "text" and }

> LayoutTests/resources/ui-helper.js:248
> +	const resolveShareSheet = `(() => uiController.invokeShareSheetWithResolution(${resolved}))()`;

Nit - Indentation is a bit off here.

[Olivia Barnett]

(In reply to Wenson Hsieh from comment #48)
> Comment on attachment 348225 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=348225&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        No new tests; adjusted expectations for existing tests.
> 
> You should mention your new test in fast/events/ios!
> 
> > Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:384
> > +- (void)invokeShareSheetWithResolution:(BOOL)resolved WK_API_AVAILABLE(ios(11.3));
> 
> This should be WK_API_AVAILABLE(ios(WK_IOS_TBA));
> 
> > Source/WebKit/UIProcess/PageClient.h:205
> > +    virtual bool showShareSheet(WebPageProxy*, const WebCore::ShareDataWithParsedURL&,  WTF::CompletionHandler<void (bool)>&&) { return false; }
> 
> Nit - extra space before WTF::
> 
> > Source/WebKit/UIProcess/WebPageProxy.cpp:5254
> > +    
> 
> Nit - it doesn't look like this extra whitespace change is intended
> 
> > Source/WebKit/UIProcess/ios/forms/WKShareSheet.h:46
> > +@property (nonatomic, weak, assign) id <WKShareSheetDelegate> delegate;
> 
> Nit - I don't think you need the assign here.
> 
> > Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:40
> > +@interface _WKShareSheetItem : NSObject
> 
> Hm...what's _WKShareSheetItem used for?

Ah, nothing.
> 
> > Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:80
> > +    RetainPtr<UIActivityViewController> shareSheetController = adoptNS([[UIActivityViewController alloc] initWithActivityItems:shareDataArray.get() applicationActivities:nil]);
> 
> Nit - s/RetainPtr<UIActivityViewController>/auto/
> 
> > Source/WebKit/UIProcess/ios/forms/WKShareSheet.mm:88
> > +    _shareSheetViewController = shareSheetController;
> 
> This can be _shareSheetViewController = WTFMove(shareSheetController);
> 
> > Source/WebKit/WebProcess/WebPage/WebPage.cpp:6085
> > +void WebPage::runShareSheetResponse(bool wasGranted, uint64_t callbackID)
> 
> s/uint64_t/ShareSheetCallbackID/ here?
> 
> > Source/WebKit/WebProcess/WebPage/WebPage.h:1734
> > +    HashMap<uint64_t, WTF::Function<void(bool completed)>> m_shareSheetResponseCallbackMap;
> 
> ShareSheetCallbackID here too?
> 
> > LayoutTests/fast/events/ios/share.html:20
> > +                    navigator.share({ title: "Example Page", url: "url", text: "text"}).then((result) => {
> 
> Nit - Indentation is a bit off here too.
> 
> Also, a space between "text" and }
> 
> > LayoutTests/resources/ui-helper.js:248
> > +	const resolveShareSheet = `(() => uiController.invokeShareSheetWithResolution(${resolved}))()`;
> 
> Nit - Indentation is a bit off here.

[Tim Horton]

Comment on attachment 348225 [details]
Patch

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

> Source/WebCore/page/NavigatorShare.idl:15
> + * 3.  Neither the name of Apple Inc. ("Apple") nor the names of
> + *     its contributors may be used to endorse or promote products derived
> + *     from this software without specific prior written permission.

This this the wrong license. Should be 2-clause BSD.

[Olivia Barnett]

Created attachment 348241 [details]
Patch

[EWS Watchlist]

Attachment 348241 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 66 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Wenson Hsieh]

Comment on attachment 348241 [details]
Patch

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

I think this is looking pretty good. Just some minor comments below:

> Source/WebCore/page/Navigator.cpp:135
> +    return;

Nit - you don't need the return down here.

> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:500
> +// Defaults to true

It seems like this only defaults to true on iOS?

> Source/WebKit/UIProcess/PageClient.h:205
> +    virtual bool showShareSheet(WebPageProxy*, const WebCore::ShareDataWithParsedURL&, WTF::CompletionHandler<void (bool)>&&) { return false; }

It doesn't seem like the first argument here (WebPageProxy*) is needed.

> Source/WebKit/UIProcess/WebPageProxy.cpp:4593
> +    CompletionHandler<void(bool)> completionHandler = [this, protectedThis = makeRef(*this), callbackID] (bool access) {

I don't think you need to capture this as well as protectedThis; you can just capture protectedThis and then use protectedThis->m_process and protectedThis->m_pageID.

> Source/WebKit/UIProcess/WebPageProxy.h:1479
> +    void runShareSheet(const WebCore::ShareDataWithParsedURL&, uint64_t callbackID);

ShareSheetCallbackID!

> Source/WebKit/WebProcess/WebPage/WebPage.h:1078
> +    void runShareSheetResponse(bool wasCompleted, uint64_t contextId);

ShareSheetCallbackID!

> LayoutTests/resources/ui-helper.js:248
> +	const resolveShareSheet = `(() => uiController.invokeShareSheetWithResolution(${resolved}))()`;

Nit - still missing one level of indentation here.

[Wenson Hsieh]

Comment on attachment 348241 [details]
Patch

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

>> Source/WebKit/UIProcess/WebPageProxy.cpp:4593
>> +    CompletionHandler<void(bool)> completionHandler = [this, protectedThis = makeRef(*this), callbackID] (bool access) {
> 
> I don't think you need to capture this as well as protectedThis; you can just capture protectedThis and then use protectedThis->m_process and protectedThis->m_pageID.

Actually, never mind! A quick grep through WebKit shows that this is a pretty common idiom.

[Olivia Barnett]

> > LayoutTests/resources/ui-helper.js:248
> > +	const resolveShareSheet = `(() => uiController.invokeShareSheetWithResolution(${resolved}))()`;
> 
> Nit - still missing one level of indentation here.

Wait I'm confused -- all the examples above (like setTimePickerValue) have the same indentation?

[Olivia Barnett]

Created attachment 348299 [details]
Patch

[EWS Watchlist]

Attachment 348299 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 67 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Comment on attachment 348299 [details]
Patch

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

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:423
> +@property (nonatomic, copy, nullable, setter=_setTitle:) NSString *_title;

Get that `nullable` out of there, it's viral and makes the compiler want more nullable in more places.

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:385
> +- (void)invokeShareSheetWithResolution:(BOOL)resolved WK_API_AVAILABLE(ios(WK_IOS_TBA));

I see other things around here don't have leading underscores, but I don't understand it. Wenson, what's up with that?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:67
> +struct ShareDataWithParsedURL;

Sort this up one line.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6074
> +void WebPage::runShareSheet(ShareDataWithParsedURL& shareData, WTF::CompletionHandler<void(bool)>&& callback)

"Run" is weird. How about s/runShareSheet/showShareSheet/.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:6085
> +void WebPage::runShareSheetResponse(bool wasGranted, ShareSheetCallbackID callbackID)

"Run" is weird. How about s/runShareSheetResponse/didCompleteShareSheet/ or didDismissShareSheet or something

> LayoutTests/resources/ui-helper.js:268
> +	const resolveShareSheet = `(() => uiController.invokeShareSheetWithResolution(${resolved}))()`;
> +	return new Promise(resolve => testRunner.runUIScript(resolveShareSheet, resolve));

The confusion here between you and Wenson is that you have TABS here. No tabs please (and this is why!!)

[Wenson Hsieh]

Comment on attachment 348299 [details]
Patch

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

>> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:385
>> +- (void)invokeShareSheetWithResolution:(BOOL)resolved WK_API_AVAILABLE(ios(WK_IOS_TBA));
> 
> I see other things around here don't have leading underscores, but I don't understand it. Wenson, what's up with that?

TBH, it seems like there should be leading underscores per WebKit SPI convention...one of us should clean this all up at some point :P

[Olivia Barnett]

Created attachment 348315 [details]
Patch

[EWS Watchlist]

Attachment 348315 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 67 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Comment on attachment 348315 [details]
Patch

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

> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:500
> +// Defaults to false, true on iOS wk2

This is WebKit2 code, so you can just drop the wk2 (it's also not called wk2 in the API so this is a weird thing to have in an SPI header).

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:385
> +- (void)invokeShareSheetWithResolution:(BOOL)resolved WK_API_AVAILABLE(ios(WK_IOS_TBA));

Still missing the underscore.

[Olivia Barnett]

Created attachment 348332 [details]
Patch

[EWS Watchlist]

Attachment 348332 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 67 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Olivia Barnett]

Created attachment 348351 [details]
Patch

[EWS Watchlist]

Attachment 348351 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 67 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 348351 [details]
Patch

Attachment 348351 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9016523

New failing tests:
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
imported/w3c/web-platform-tests/web-share/share-securecontext.http.html

[EWS Watchlist]

Created attachment 348366 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[Olivia Barnett]

Created attachment 348367 [details]
Patch

[EWS Watchlist]

Attachment 348367 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 70 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 348367 [details]
Patch

Attachment 348367 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9018140

New failing tests:
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
imported/w3c/web-platform-tests/web-share/share-securecontext.http.html

[EWS Watchlist]

Created attachment 348377 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348367 [details]
Patch

Attachment 348367 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9018379

New failing tests:
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
imported/w3c/web-platform-tests/web-share/share-securecontext.http.html

[EWS Watchlist]

Created attachment 348378 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348367 [details]
Patch

Attachment 348367 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9018120

New failing tests:
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html

[EWS Watchlist]

Created attachment 348379 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[EWS Watchlist]

Comment on attachment 348367 [details]
Patch

Attachment 348367 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9018878

New failing tests:
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html
imported/w3c/web-platform-tests/web-share/share-securecontext.http.html

[EWS Watchlist]

Created attachment 348386 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348367 [details]
Patch

Attachment 348367 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/9018997

New failing tests:
imported/w3c/web-platform-tests/web-share/share-url-invalid.https.html

[EWS Watchlist]

Created attachment 348391 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[Olivia Barnett]

Created attachment 348442 [details]
Patch

[EWS Watchlist]

Attachment 348442 [details] did not pass style-queue:


ERROR: Source/WebCore/page/ChromeClient.h:286:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 68 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 348442 [details]
Patch

Clearing flags on attachment: 348442

Committed r235489: <https://trac.webkit.org/changeset/235489>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

Looks like after https://trac.webkit.org/changeset/235489/webkit

fast/dom/navigator-detached-no-crash.html is now crashing constantly

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fdom%2Fnavigator-detached-no-crash.html

Crash log:
https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)/r235489%20(6178)/fast/dom/navigator-detached-no-crash-crash-log.txt

[Tim Horton]

Super weird that we didn't get that here.

[David Kilzer (:ddkilzer)]

(In reply to WebKit Commit Bot from comment #83)
> Comment on attachment 348442 [details]
> Patch
> 
> Clearing flags on attachment: 348442
> 
> Committed r235489: <https://trac.webkit.org/changeset/235489>

Follow-up fix due to bad WebKit Xcode project merge:

Committed r235710: <https://trac.webkit.org/changeset/235710>