[webnfc] Support multiple scan invocations on same Reader

Bug: 520391
Change-Id: I2dc1160947ea6b97ee5dc7e5d5cf5ca57e57b1e2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2145263
Commit-Queue: Donna Wu <donna.wu@intel.com>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759129}

third_party/blink/renderer/modules/nfc/ndef_reader.cc

@@ -92,24 +92,21 @@ ScriptPromise NDEFReader::scan(ScriptState* script_state,
     return ScriptPromise();
   }
 
-  // TODO(https://crbug.com/520391): With the note in
+  // With the note in
   // https://w3c.github.io/web-nfc/#the-ndefreader-and-ndefwriter-objects,
   // successive invocations of NDEFReader.scan() with new options should replace
-  // existing filters. For now we just reject this new scan() when there is an
-  // ongoing filter active.
+  // existing filters. So stop current reading for this case.
   if (GetNfcProxy()->IsReading(this)) {
-    exception_state.ThrowDOMException(
-        DOMExceptionCode::kInvalidStateError,
-        "There is already a scan() operation ongoing.");
-    return ScriptPromise();
+    Abort(signal_.Get());
   }
 
   resolver_ = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   // 8. If reader.[[Signal]] is not null, then add the following abort steps to
   // reader.[[Signal]]:
   if (options->hasSignal()) {
-    options->signal()->AddAlgorithm(
-        WTF::Bind(&NDEFReader::Abort, WrapPersistent(this)));
+    signal_ = options->signal();
+    signal_->AddAlgorithm(WTF::Bind(&NDEFReader::Abort, WrapPersistent(this),
+                                    WrapPersistent(options->signal())));
   }
 
   GetPermissionService()->RequestPermission(
@@ -173,6 +170,7 @@ void NDEFReader::OnScanRequestCompleted(
 
 void NDEFReader::Trace(Visitor* visitor) {
   visitor->Trace(resolver_);
+  visitor->Trace(signal_);
   EventTargetWithInlineData::Trace(visitor);
   ActiveScriptWrappable::Trace(visitor);
   ExecutionContextLifecycleObserver::Trace(visitor);
@@ -216,7 +214,14 @@ void NDEFReader::ContextDestroyed() {
   GetNfcProxy()->StopReading(this);
 }
 
-void NDEFReader::Abort() {
+void NDEFReader::Abort(AbortSignal* signal) {
+  // In the case of successive invocations of NDEFReader.scan() with
+  // different signals, we should make sure aborting on previous signal
+  // won't abort current reading.
+  // If this is not triggered by the current signal, just ignore it.
+  if (signal && signal != signal_)
+    return;
+
   if (resolver_) {
     resolver_->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kAbortError, "The NFC operation was cancelled."));

third_party/blink/renderer/modules/nfc/ndef_reader.h

@@ -17,6 +17,7 @@
 
 namespace blink {
 
+class AbortSignal;
 class ExecutionContext;
 class NFCProxy;
 class NDEFScanOptions;
@@ -59,7 +60,7 @@ class MODULES_EXPORT NDEFReader : public EventTargetWithInlineData,
   // ExecutionContextLifecycleObserver overrides.
   void ContextDestroyed() override;
 
-  void Abort();
+  void Abort(AbortSignal* signal);
 
   NFCProxy* GetNfcProxy() const;
 
@@ -75,6 +76,11 @@ class MODULES_EXPORT NDEFReader : public EventTargetWithInlineData,
   // case the callback passed to Watch() won't be called and
   // mojo::WrapCallbackWithDefaultInvokeIfNotRun() is forbidden in Blink.
   Member<ScriptPromiseResolver> resolver_;
+
+  // Currently AbortSignal has no method to remove an algorithm so this
+  // field tracks the most recently configured AbortSignal so that others
+  // can be ignored.
+  Member<AbortSignal> signal_;
 };
 
 }  // namespace blink

third_party/blink/web_tests/external/wpt/web-nfc/NDEFReader_scan_filter.https-expected.txt

-This is a testharness.js-based test.
-PASS Test that reading data succeed when NDEFScanOptions' recordType is set to 'empty'.
-PASS Test that reading data succeed when NDEFScanOptions' recordType is set to 'mime'.
-PASS Test that reading data succeed when NDEFScanOptions' recordType is set to 'unknown'.
-PASS Test that reading data succeed when NDEFScanOptions' recordType is set to 'text'.
-PASS Test that reading data succeed when NDEFScanOptions' recordType is set to 'url'.
-PASS Test that reading data succeed when NDEFScanOptions' recordType is set to 'absolute-url'.
-PASS Test that reading data succeed when NDEFScanOptions' recordType is set to a custom type for external type records.
-PASS Test that the id of NDEFScanOptions filters relevant data sources correctly.
-PASS Test that the mediaType of NDEFScanOptions filters relevant data sources correctly.
-PASS Test that filtering 'empty' record from different messages correctly with NDEFScanOptions' recordType is set to 'empty'.
-PASS Test that filtering 'mime' record from different messages correctly with NDEFScanOptions' recordType is set to 'mime'.
-PASS Test that filtering 'unknown' record from different messages correctly with NDEFScanOptions' recordType is set to 'unknown'.
-PASS Test that filtering 'text' record from different messages correctly with NDEFScanOptions' recordType is set to 'text'.
-PASS Test that filtering 'url' record from different messages correctly with NDEFScanOptions' recordType is set to 'url'.
-PASS Test that filtering 'absolute-url' record from different messages correctly with NDEFScanOptions' recordType is set to 'absolute-url'.
-PASS Test that filtering external record from different messages correctly with NDEFScanOptions' recordType is set to the custom type.
-PASS Test that filtering 'text' record from different messages correctly with NDEFScanOptions' id set.
-PASS Test that filtering 'mime' record from different messages correctly with NDEFScanOptions' mediaType set.
-FAIL Multiple scan() from the same NDEFReader object with new options should replace existing filters. promise_test: Unhandled rejection with value: object "InvalidStateError: Failed to execute 'scan' on 'NDEFReader': There is already a scan() operation ongoing."
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/web-nfc/NDEFReader_scan_filter.https.html

@@ -176,9 +176,9 @@ nfc_test(async (t, mockNFC) => {
     assertWebNDEFMessagesEqual(event.message, new NDEFMessage(mimeMsg));
   });
 
-  const scanOptions1 = { recordType: "text", signal: signal };
-  const scanOptions2 = {recordType: "url", signal: signal };
-  const scanOptions3 = {recordType: "mime", signal: signal };
+  const scanOptions1 = {recordType: "text", signal: signal};
+  const scanOptions2 = {recordType: "url", signal: signal};
+  const scanOptions3 = {recordType: "mime", signal: signal};
 
   await reader.scan(scanOptions1);
   await reader.scan(scanOptions2);
@@ -193,4 +193,48 @@ nfc_test(async (t, mockNFC) => {
 }, "Multiple scan() from the same NDEFReader object with new options should \
 replace existing filters.");
 
+nfc_test(async (t, mockNFC) => {
+  const reader = new NDEFReader();
+  const controller1 = new AbortController();
+  const controller2 = new AbortController();
+  const urlMsg = createMessage([createUrlRecord(test_url_data)]);
+  const mimeMsg = createMessage([createMimeRecord(test_buffer_data)]);
+
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+  const promise = readerWatcher.wait_for("reading").then(event => {
+    assertWebNDEFMessagesEqual(event.message, new NDEFMessage(mimeMsg));
+    controller2.abort();
+  });
+
+  const scanOptions1 = {recordType: "url", signal: controller1.signal};
+  const scanOptions2 = {recordType: "mime", signal: controller2.signal};
+
+  // There is maximum one filter for an NDEFReader object,
+  // last filter will replace all previous ones.
+  await reader.scan(scanOptions1);
+  await reader.scan(scanOptions2);
+
+  mockNFC.setReadingMessage(urlMsg);
+
+  controller1.abort();
+
+  mockNFC.setReadingMessage(mimeMsg);
+  await promise;
+}, "Aborting on previous signal should not stop current reading for multiple \
+scan() with different signals.");
+
+nfc_test(async (t, mockNFC) => {
+  const reader = new NDEFReader();
+  const controller = new AbortController();
+  const scanOptions1 = {recordType: "url", signal: controller.signal };
+  const scanOptions2 = {recordType: "mime"};
+
+  await reader.scan(scanOptions1);
+  const promise = reader.scan(scanOptions2);
+  controller.abort();
+
+  await promise_rejects_dom(t, 'AbortError', promise);
+}, "Aborting on previous signal can stop current reading if no new signals \
+passed to successive scan().");
+
 </script>