[webnfc] Reject the push when a non-NDEF tag comes in proximity

Previously, when a non-NDEF tag comes in proximity, NDEFWriter#push()
does nothing and still keeps in pending state.

This CL switches to reject NDEFWriter#push() in such case so that WebDev
can get a sense of what's happening.

BUG=520391

Change-Id: I17569910466e75f12ff4e37c29180e2a9af9db13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1967745
Commit-Queue: Leon Han <leon.han@intel.com>
Reviewed-by: François Beaufort <beaufort.francois@gmail.com>
Reviewed-by: Rijubrata Bhaumik <rijubrata.bhaumik@intel.com>
Cr-Commit-Position: refs/heads/master@{#726643}

services/device/nfc/android/java/src/org/chromium/device/nfc/NfcImpl.java

@@ -630,7 +630,9 @@ public class NfcImpl implements Nfc {
 
         // This tag is not NDEF compatible.
         if (mTagHandler == null) {
+            Log.w(TAG, "This tag is not NDEF compatible.");
             notifyErrorToAllWatchers(NdefErrorType.NOT_SUPPORTED);
+            pendingPushOperationCompleted(createError(NdefErrorType.NOT_SUPPORTED));
             return;
         }
 

services/device/nfc/android/junit/src/org/chromium/device/nfc/NFCTest.java

@@ -831,7 +831,7 @@ public class NFCTest {
 
     /**
      * Test that when the tag in proximity is found to be not NDEF compatible, an error event will
-     * be dispatched to the client.
+     * be dispatched to the client and the pending push operation will also be ended with an error.
      */
     @Test
     @Feature({"NFCTest"})
@@ -842,6 +842,9 @@ public class NFCTest {
         // Prepare at least one watcher, otherwise the error won't be notified.
         WatchResponse mockWatchCallback = mock(WatchResponse.class);
         nfc.watch(createNdefScanOptions(), mNextWatchId, mockWatchCallback);
+        // Start a push.
+        PushResponse mockCallback = mock(PushResponse.class);
+        nfc.push(createMojoNdefMessage(), createNdefPushOptions(), mockCallback);
 
         // Pass null tag handler to simulate that the tag is not NDEF compatible.
         nfc.processPendingOperationsForTesting(null);
@@ -852,6 +855,11 @@ public class NFCTest {
         verify(mNfcClient, times(0))
                 .onWatch(mOnWatchCallbackCaptor.capture(), nullable(String.class),
                         any(NdefMessage.class));
+
+        // The pending push failed with the correct error.
+        verify(mockCallback).call(mErrorCaptor.capture());
+        assertNotNull(mErrorCaptor.getValue());
+        assertEquals(NdefErrorType.NOT_SUPPORTED, mErrorCaptor.getValue().errorType);
     }
 
     /**

third_party/blink/web_tests/external/wpt/resources/chromium/nfc-mock.js

@@ -154,14 +154,13 @@ var WebNFCTest = (() => {
 
       this.hw_status_ = NFCHWStatus.ENABLED;
       this.pushed_message_ = null;
-      this.push_options_ = null;
+      this.pending_push_options_ = null;
       this.pending_promise_func_ = null;
       this.push_completed_ = true;
       this.client_ = null;
       this.watchers_ = [];
       this.reading_messages_ = [];
       this.operations_suspended_ = false;
-      this.is_ndef_tech_ = true;
       this.is_formatted_tag_ = false;
     }
 
@@ -176,18 +175,11 @@ var WebNFCTest = (() => {
       }
 
       this.pushed_message_ = message;
-      this.push_options_ = options;
-
+      this.pending_push_options_ = options;
       return new Promise(resolve => {
-        this.pending_promise_func_ = resolve;
-        if (this.operations_suspended_) {
-          // Pends push operation if NFC operation is suspended.
-        } else if (!this.push_completed_) {
-          // Leaves the push operating pending.
-        } else if (!this.is_ndef_tech_) {
-          // Resolves with NotSupportedError if the device does not expose
-          // NDEF technology.
-          resolve(createNDEFError(device.mojom.NDEFErrorType.NOT_SUPPORTED));
+        if (this.operations_suspended_ || !this.push_completed_) {
+          // Leaves the push pending.
+          this.pending_promise_func_ = resolve;
         } else if (this.is_formatted_tag_ && !options.overwrite) {
           // Resolves with NotAllowedError if there are NDEF records on the device
           // and overwrite is false.
@@ -199,8 +191,9 @@ var WebNFCTest = (() => {
     }
 
     async cancelPush(target) {
-      if (this.push_options_ && ((target === device.mojom.NDEFPushTarget.ANY) ||
-          (this.push_options_.target === target))) {
+      if (this.pending_push_options_ &&
+          ((target === device.mojom.NDEFPushTarget.ANY) ||
+           (this.pending_push_options_.target === target))) {
         this.cancelPendingPushOperation();
       }
 
@@ -221,7 +214,7 @@ var WebNFCTest = (() => {
       this.watchers_.push({id: id, options: options});
       // Ignores reading if NFC operation is suspended
       // or the NFC tag does not expose NDEF technology.
-      if(!this.operations_suspended_ && this.is_ndef_tech_) {
+      if (!this.operations_suspended_) {
         // Triggers onWatch if the new watcher matches existing messages.
         for (let message of this.reading_messages_) {
           if (matchesWatchOptions(message, options)) {
@@ -270,7 +263,7 @@ var WebNFCTest = (() => {
     }
 
     pushOptions() {
-      return this.push_options_;
+      return this.pending_push_options_;
     }
 
     watchOptions() {
@@ -288,7 +281,6 @@ var WebNFCTest = (() => {
       this.reading_messages_ = [];
       this.operations_suspended_ = false;
       this.cancelPendingPushOperation();
-      this.is_ndef_tech_ = true;
       this.is_formatted_tag_ = false;
     }
 
@@ -296,23 +288,21 @@ var WebNFCTest = (() => {
       if (this.pending_promise_func_) {
         this.pending_promise_func_(
             createNDEFError(device.mojom.NDEFErrorType.OPERATION_CANCELLED));
+        this.pending_promise_func_ = null;
       }
 
       this.pushed_message_ = null;
-      this.push_options_ = null;
-      this.pending_promise_func_ = null;
+      this.pending_push_options_ = null;
       this.push_completed_ = true;
     }
 
     // Sets message that is used to deliver NFC reading updates.
     setReadingMessage(message) {
       this.reading_messages_.push(message);
-      // Ignores reading if the NFC tag does not expose NDEF technology.
-      if(!this.is_ndef_tech_) return;
       // Ignores reading if NFC operation is suspended.
       if(this.operations_suspended_) return;
       // Ignores reading if NDEFPushOptions.ignoreRead is true.
-      if(this.push_options_ && this.push_options_.ignoreRead)
+      if (this.pending_push_options_ && this.pending_push_options_.ignoreRead)
         return;
       // Triggers onWatch if the new message matches existing watchers.
       for (let watcher of this.watchers_) {
@@ -336,7 +326,7 @@ var WebNFCTest = (() => {
       // Resumes pending NFC reading.
       for (let watcher of this.watchers_) {
         for (let message of this.reading_messages_) {
-          if (matchesWatchOptions(message, watcher.options) && this.is_ndef_tech_) {
+          if (matchesWatchOptions(message, watcher.options)) {
             this.client_.onWatch(
                 [watcher.id], fake_tag_serial_number,
                 toMojoNDEFMessage(message));
@@ -344,16 +334,24 @@ var WebNFCTest = (() => {
         }
       }
       // Resumes pending push operation.
-      if (this.pending_promise_func_) {
+      if (this.pending_promise_func_ && this.push_completed_) {
         this.pending_promise_func_(createNDEFError(null));
+        this.pending_promise_func_ = null;
       }
     }
 
-    setIsNDEFTech(isNdef) {
-      this.is_ndef_tech_ = isNdef;
-      if (!isNdef && this.watchers_.length != 0) {
+    // Simulates the device coming in proximity does not expose NDEF technology.
+    simulateNonNDEFTagDiscovered() {
+      // Notify NotSupportedError to all active readers.
+      if (this.watchers_.length != 0) {
         this.client_.onError(device.mojom.NDEFErrorType.NOT_SUPPORTED);
       }
+      // Reject the pending push with NotSupportedError.
+      if (this.pending_promise_func_) {
+        this.pending_promise_func_(
+            createNDEFError(device.mojom.NDEFErrorType.NOT_SUPPORTED));
+        this.pending_promise_func_ = null;
+      }
     }
 
     setIsFormattedTag(isFormatted) {

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

@@ -186,7 +186,7 @@ nfc_test(async (t, mockNFC) => {
   promises.push(promise2);
   await reader2.scan();
 
-  mockNFC.setIsNDEFTech(false);
+  mockNFC.simulateNonNDEFTagDiscovered();
   await Promise.all(promises);
 }, "Test that NDEFReader.onerror should be fired if the NFC tag does not \
 expose NDEF technology.");

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

@@ -26,7 +26,7 @@ PASS NDEFWriter.push should replace all previously configured push operations.
 PASS Test that recordType should be set to 'text' if NDEFRecordInit.record's recordType is undefined and NDEFRecordInit.record's data is DOMString.
 PASS Test that recordType should be set to 'mime' if NDEFRecordInit.record's recordType is undefined and NDEFRecordInit.record's data is not DOMString.
 PASS Test that mediaType should be set to 'application/octet-stream' if NDEFRecordInit.record's recordType is 'mime' and NDEFRecordInit.record's mediaType is undefined.
-PASS NDEFWriter.push should fail when the NFC device does not expose NDEF technology.
+PASS NDEFWriter.push should fail when the NFC device coming up does not expose NDEF technology.
 PASS NDEFWriter.push should succeed to push data to an unformatted NFC device when the NDEFPushOptions.overwrite is false.
 PASS NDEFWriter.push should succeed to overwrite the existing data when the NDEFPushOptions.overwrite is true.
 FAIL NDEFWriter.push should fail when there are NDEF records on the NFC device and NDEFPushOptions.overwrite is false. assert_unreached: Should have rejected: undefined Reached unreachable code

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

@@ -408,10 +408,19 @@ NDEFRecordInit.record's recordType is 'mime' and NDEFRecordInit.record's \
 mediaType is undefined.");
 
 nfc_test(async (t, mockNFC) => {
+  // Make sure the push will be pending in the mock.
+  mockNFC.setPendingPushCompleted(false);
+
   const writer = new NDEFWriter();
-  mockNFC.setIsNDEFTech(false);
-  await promise_rejects(t, 'NotSupportedError', writer.push(test_text_data));
-}, "NDEFWriter.push should fail when the NFC device does not expose \
+  const promise = writer.push(test_text_data);
+
+  // Just to make sure the push() request has already reached to the mock.
+  const reader = new NDEFReader();
+  await reader.scan();
+
+  mockNFC.simulateNonNDEFTagDiscovered();
+  await promise_rejects(t, 'NotSupportedError', promise);
+}, "NDEFWriter.push should fail when the NFC device coming up does not expose \
 NDEF technology.");
 
 nfc_test(async (t, mockNFC) => {

third_party/blink/web_tests/external/wpt/web-nfc/README.md

@@ -16,7 +16,7 @@ The `WebNFCTest` interface is defined as:
     setPendingPushCompleted(boolean result); // Sets if the pending push is completed.
     pushedMessage(); // Gets the pushed `NDEFMessageSource`.
     pushOptions(); // Gets the pushed `NDEFPushOptions`.
-    setIsNDEFTech(boolean isNDEF); // Sets if the NFC device exposes NDEF technology.
+    simulateNonNDEFTagDiscovered(); // Simulates that the NFC device discovered does not expose NDEF technology.
     setIsFormattedTag(boolean isFormatted); // Sets if the NFC tag has formatted NDEF message.
   };
 ```