Web Share: Only allow one share() call at a time

We update behavior to match spec change
https://github.com/w3c/web-share/pull/113

If a share() call is in progress and share() is called again,
the new share request fails immediately.

WebShare for ChromeOS and Windows will only ever have this
new behavior. WebShare for Android (already shipped)
would previously spin up multiple shares simultaneously.

Spec:
https://w3c.github.io/web-share/#share-method


Note: we no longer have an Android post Lollipop MR1
test for user cancellation of the dialog - the existing
test simply issues a second share request.

Bug: 1002337
Change-Id: I1bef4923cb048a28f882f9dac889c841f1fc8265
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425933Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811265}

chrome/android/javatests/src/org/chromium/chrome/browser/webshare/WebShareTest.java

@@ -137,15 +137,17 @@ public class WebShareTest {
     @Feature({"WebShare"})
     @Features.DisableFeatures(ChromeFeatureList.CHROME_SHARING_HUB)
     @MinAndroidSdkLevel(Build.VERSION_CODES.LOLLIPOP_MR1)
-    public void testWebShareCancel() throws Exception {
-        // Set up ShareHelper to ignore the intent (without showing a picker). This simulates the
-        // user canceling the dialog.
+    public void testWebShareDoubleRequest() throws Exception {
+        // Set up ShareHelper to ignore the intent (without showing a picker),
+        // and request another share.
         ShareHelper.setFakeIntentReceiverForTesting(new FakeIntentReceiverPostLMR1(false));
 
         mActivityTestRule.loadUrl(mTestServer.getURL(TEST_FILE));
         // Click (instead of directly calling the JavaScript function) to simulate a user gesture.
         TouchCommon.singleClickView(mTab.getView());
-        Assert.assertEquals("Fail: AbortError: Share canceled", mUpdateWaiter.waitForUpdate());
+        Assert.assertEquals("Fail: InvalidStateError: Failed to execute 'share' on 'Navigator': "
+                        + "A earlier share had not yet completed.",
+                mUpdateWaiter.waitForUpdate());
     }
 
     /**
@@ -458,10 +460,8 @@ public class WebShareTest {
             mReceivedIntent = intent;
 
             if (!mProceed) {
-                // Click again to start another share, which cancels the current share.
-                // This is necessary to work around https://crbug.com/636274 (callback
-                // is not canceled until next share is initiated).
-                // This also serves as a regression test for https://crbug.com/640324.
+                // Click again to start another share, which fails as a share is already in
+                // progress.
                 TouchCommon.singleClickView(mTab.getView());
                 return;
             }

third_party/blink/renderer/modules/webshare/navigator_share.cc

@@ -133,8 +133,10 @@ NavigatorShare::ShareClientImpl::ShareClientImpl(
                   {SchedulingPolicy::RecordMetricsForBackForwardCache()})) {}
 
 void NavigatorShare::ShareClientImpl::Callback(mojom::blink::ShareError error) {
-  if (navigator_)
-    navigator_->clients_.erase(this);
+  if (navigator_) {
+    DCHECK_EQ(navigator_->client_, this);
+    navigator_->client_ = nullptr;
+  }
 
   if (error == mojom::blink::ShareError::OK) {
     UseCounter::Count(ExecutionContext::From(resolver_->GetScriptState()),
@@ -173,7 +175,7 @@ NavigatorShare& NavigatorShare::From(Navigator& navigator) {
 
 void NavigatorShare::Trace(Visitor* visitor) const {
   visitor->Trace(service_remote_);
-  visitor->Trace(clients_);
+  visitor->Trace(client_);
   Supplement<Navigator>::Trace(visitor);
 }
 
@@ -213,6 +215,12 @@ ScriptPromise NavigatorShare::share(ScriptState* script_state,
           ? WebFeature::kWebSharePolicyAllow
           : WebFeature::kWebSharePolicyDisallow);
 
+  if (client_) {
+    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
+                                      "A earlier share had not yet completed.");
+    return ScriptPromise();
+  }
+
   if (!LocalFrame::ConsumeTransientUserActivation(window->GetFrame())) {
     exception_state.ThrowDOMException(
         DOMExceptionCode::kNotAllowedError,
@@ -238,10 +246,7 @@ ScriptPromise NavigatorShare::share(ScriptState* script_state,
 
   bool has_files = HasFiles(*data);
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
-  ShareClientImpl* client =
-      MakeGarbageCollected<ShareClientImpl>(this, has_files, resolver);
-  clients_.insert(client);
-  ScriptPromise promise = resolver->Promise();
+  client_ = MakeGarbageCollected<ShareClientImpl>(this, has_files, resolver);
 
   WTF::Vector<mojom::blink::SharedFilePtr> files;
   uint64_t total_bytes = 0;
@@ -264,9 +269,9 @@ ScriptPromise NavigatorShare::share(ScriptState* script_state,
   service_remote_->Share(
       data->hasTitle() ? data->title() : g_empty_string,
       data->hasText() ? data->text() : g_empty_string, url, std::move(files),
-      WTF::Bind(&ShareClientImpl::Callback, WrapPersistent(client)));
+      WTF::Bind(&ShareClientImpl::Callback, WrapPersistent(client_.Get())));
 
-  return promise;
+  return resolver->Promise();
 }
 
 ScriptPromise NavigatorShare::share(ScriptState* script_state,
@@ -277,10 +282,10 @@ ScriptPromise NavigatorShare::share(ScriptState* script_state,
 }
 
 void NavigatorShare::OnConnectionError() {
-  for (auto& client : clients_) {
-    client->OnConnectionError();
+  if (client_) {
+    client_->OnConnectionError();
+    client_ = nullptr;
   }
-  clients_.clear();
   service_remote_.reset();
 }
 

third_party/blink/renderer/modules/webshare/navigator_share.h

@@ -56,7 +56,8 @@ class MODULES_EXPORT NavigatorShare final
   // |NavigatorShare| is not ExecutionContext-associated.
   HeapMojoRemote<blink::mojom::blink::ShareService> service_remote_{nullptr};
 
-  HeapHashSet<Member<ShareClientImpl>> clients_;
+  // Represents a user's current intent to share some data.
+  Member<ShareClientImpl> client_ = nullptr;
 };
 
 }  // namespace blink

third_party/blink/web_tests/webshare/share-error.html

@@ -63,4 +63,26 @@ share_test(mock => {
   });
 }, 'share consumes user activation');
 
+share_test(mock => {
+  mock.pushShareResult('the title', 'the message', 'https://example.com/',
+                       blink.mojom.ShareError.CANCELED);
+  return callWithKeyDown(async () => {
+    const data = {
+          title: 'the title',
+          text: 'the message',
+          url: 'https://example.com/'
+    };
+    const first = navigator.share(data);
+    await assertRejectsWithError(
+          navigator.share(data),
+          'InvalidStateError');
+    await assertRejectsWithError(
+          navigator.share(data),
+          'InvalidStateError');
+    return assertRejectsWithError(
+          first,
+          'AbortError');
+  } );
+}, 'only one share at a time');
+
 </script>