[webnfc] Dispatch error events when failed to read a nfc tag

Trigger NDEFReader#onerror on all active readers when we found that the
nfc tag coming nearby is not NDEF compatible or some IO errors happened
when trying to read the tag, etc.

BUG=520391

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

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

@@ -544,10 +544,22 @@ public class NfcImpl implements Nfc {
             notifyMatchingWatchers(webNdefMessage);
         } catch (UnsupportedEncodingException e) {
             Log.w(TAG, "Cannot read data from NFC tag. Cannot convert to NdefMessage.");
+            notifyErrorToAllWatchers(NdefErrorType.INVALID_MESSAGE);
         } catch (TagLostException e) {
             Log.w(TAG, "Cannot read data from NFC tag. Tag is lost.");
+            notifyErrorToAllWatchers(NdefErrorType.IO_ERROR);
         } catch (FormatException | IllegalStateException | IOException e) {
             Log.w(TAG, "Cannot read data from NFC tag. IO_ERROR.");
+            notifyErrorToAllWatchers(NdefErrorType.IO_ERROR);
+        }
+    }
+
+    /**
+     * Notify all active watchers that an error happened when trying to read the tag coming nearby.
+     */
+    private void notifyErrorToAllWatchers(int error) {
+        for (int i = 0; i < mWatchers.size(); i++) {
+            mClient.onError(error);
         }
     }
 
@@ -615,6 +627,13 @@ public class NfcImpl implements Nfc {
      */
     protected void processPendingOperations(NfcTagHandler tagHandler) {
         mTagHandler = tagHandler;
+
+        // This tag is not NDEF compatible.
+        if (mTagHandler == null) {
+            notifyErrorToAllWatchers(NdefErrorType.NOT_SUPPORTED);
+            return;
+        }
+
         processPendingWatchOperations();
         processPendingPushOperation();
         if (mTagHandler != null && mTagHandler.isConnected()) {

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

@@ -829,6 +829,53 @@ public class NFCTest {
         assertEquals(NdefErrorType.NOT_FOUND, mErrorCaptor.getValue().errorType);
     }
 
+    /**
+     * Test that when the tag in proximity is found to be not NDEF compatible, an error event will
+     * be dispatched to the client.
+     */
+    @Test
+    @Feature({"NFCTest"})
+    public void testNonNdefCompatibleTagFound() {
+        TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate);
+        mDelegate.invokeCallback();
+        nfc.setClient(mNfcClient);
+        // Prepare at least one watcher, otherwise the error won't be notified.
+        WatchResponse mockWatchCallback = mock(WatchResponse.class);
+        nfc.watch(createNdefScanOptions(), mNextWatchId, mockWatchCallback);
+
+        // Pass null tag handler to simulate that the tag is not NDEF compatible.
+        nfc.processPendingOperationsForTesting(null);
+
+        // An error is notified.
+        verify(mNfcClient, times(1)).onError(NdefErrorType.NOT_SUPPORTED);
+        // No watch.
+        verify(mNfcClient, times(0))
+                .onWatch(mOnWatchCallbackCaptor.capture(), nullable(String.class),
+                        any(NdefMessage.class));
+    }
+
+    /**
+     * Test that when the tag in proximity is found to be not NDEF compatible, an error event will
+     * not be dispatched to the client if there is no watcher present.
+     */
+    @Test
+    @Feature({"NFCTest"})
+    public void testNonNdefCompatibleTagFoundWithoutWatcher() {
+        TestNfcImpl nfc = new TestNfcImpl(mContext, mDelegate);
+        mDelegate.invokeCallback();
+        nfc.setClient(mNfcClient);
+
+        // Pass null tag handler to simulate that the tag is not NDEF compatible.
+        nfc.processPendingOperationsForTesting(null);
+
+        // An error is NOT notified.
+        verify(mNfcClient, times(0)).onError(NdefErrorType.NOT_SUPPORTED);
+        // No watch.
+        verify(mNfcClient, times(0))
+                .onWatch(mOnWatchCallbackCaptor.capture(), nullable(String.class),
+                        any(NdefMessage.class));
+    }
+
     /**
      * Test that when tag is disconnected during read operation, IllegalStateException is handled.
      */
@@ -847,10 +894,11 @@ public class NFCTest {
         // Mocks 'NFC tag found' event.
         nfc.processPendingOperationsForTesting(mNfcTagHandler);
 
-        // Check that client was not notified.
+        // Check that the watch was not triggered but an error was dispatched to the client.
         verify(mNfcClient, times(0))
                 .onWatch(mOnWatchCallbackCaptor.capture(), nullable(String.class),
                         any(NdefMessage.class));
+        verify(mNfcClient, times(1)).onError(NdefErrorType.IO_ERROR);
     }
 
     /**

services/device/public/mojom/nfc.mojom

@@ -128,4 +128,8 @@ interface NFCClient {
   // anti-collision and identification, or empty string in case none is
   // available.
   OnWatch(array<uint32> watch_ids, string? serial_number, NDEFMessage message);
+
+  // Sends |error| to all readers that are trying to read some data from the nfc
+  // tag coming nearby.
+  OnError(NDEFErrorType error);
 };

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

@@ -166,6 +166,13 @@ void NDEFReader::OnReading(const String& serial_number,
       MakeGarbageCollected<NDEFMessage>(message)));
 }
 
+void NDEFReader::OnError(device::mojom::blink::NDEFErrorType error) {
+  ErrorEvent* event = ErrorEvent::Create(
+      NDEFErrorTypeToDOMException(error)->message(),
+      SourceLocation::Capture(GetExecutionContext()), nullptr);
+  DispatchEvent(*event);
+}
+
 void NDEFReader::OnMojoConnectionError() {
   // If |resolver_| has already settled this rejection is silently ignored.
   if (resolver_) {
@@ -174,10 +181,7 @@ void NDEFReader::OnMojoConnectionError() {
   }
 
   // Dispatches an error event.
-  ErrorEvent* error = ErrorEvent::Create(
-      "No NFC adapter or cannot establish connection.",
-      SourceLocation::Capture(GetExecutionContext()), nullptr);
-  DispatchEvent(*error);
+  OnError(device::mojom::blink::NDEFErrorType::NOT_SUPPORTED);
 }
 
 void NDEFReader::ContextDestroyed(ExecutionContext*) {

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

@@ -50,6 +50,7 @@ class MODULES_EXPORT NDEFReader : public EventTargetWithInlineData,
   // Called by NFCProxy for dispatching events.
   virtual void OnReading(const String& serial_number,
                          const device::mojom::blink::NDEFMessage&);
+  virtual void OnError(device::mojom::blink::NDEFErrorType);
 
   // Called by NFCProxy for notification about connection error.
   void OnMojoConnectionError();

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

@@ -108,10 +108,9 @@ void NFCProxy::OnWatch(const Vector<uint32_t>& watch_ids,
                        const String& serial_number,
                        device::mojom::blink::NDEFMessagePtr message) {
   // Dispatch the event to all matched readers. We iterate on a copy of
-  // |readers_| because the user's NDEFReader#onreading event handler may call
-  // NDEFReader#stop() to modify |readers_| just during the iteration process.
-  // This loop is O(n^2), however, we assume the number of readers to be small
-  // so it'd be just OK.
+  // |readers_| because a reader's onreading event handler may remove itself
+  // from |readers_| just during the iteration process. This loop is O(n^2),
+  // however, we assume the number of readers to be small so it'd be just OK.
   ReaderMap copy = readers_;
   for (auto& pair : copy) {
     if (watch_ids.Contains(pair.value))
@@ -119,6 +118,16 @@ void NFCProxy::OnWatch(const Vector<uint32_t>& watch_ids,
   }
 }
 
+void NFCProxy::OnError(device::mojom::blink::NDEFErrorType error) {
+  // Dispatch the event to all readers. We iterate on a copy of |readers_|
+  // because a reader's onerror event handler may remove itself from |readers_|
+  // just during the iteration process.
+  ReaderMap copy = readers_;
+  for (auto& pair : copy) {
+    pair.key->OnError(error);
+  }
+}
+
 void NFCProxy::OnReaderRegistered(
     NDEFReader* reader,
     uint32_t watch_id,
@@ -179,6 +188,21 @@ void NFCProxy::EnsureMojoConnection() {
       client_receiver_.BindNewPipeAndPassRemote(task_runner));
 }
 
+// Once the NFC Mojo connection is established, this OnMojoConnectionError()
+// could happen in only one case: DeviceService shutdown. As currently
+// DeviceService is running in the browser process and only goes to shutdown
+// when the browser process exits, so this case should just be impossible and
+// meaningless.
+//
+// But, it's possible that in the future we may configure DeviceService to run
+// in some separate process and may start/stop/start it under some conditions
+// (e.g. handle some unexpected crashes), then each time DeviceService goes down
+// we will get this OnMojoConnectionError().
+//
+// However, for now, this OnMojoConnectionError() happens only when we failed to
+// establish the NFC Mojo connection in the first place, i.e. the connection
+// request is rejected by the browser side (DeviceService) due to missing NFC
+// support etc.
 void NFCProxy::OnMojoConnectionError() {
   nfc_remote_.reset();
   client_receiver_.reset();

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

@@ -60,6 +60,7 @@ class MODULES_EXPORT NFCProxy final : public GarbageCollected<NFCProxy>,
   void OnWatch(const Vector<uint32_t>&,
                const String&,
                device::mojom::blink::NDEFMessagePtr) override;
+  void OnError(device::mojom::blink::NDEFErrorType) override;
 
   void OnReaderRegistered(NDEFReader*,
                           uint32_t watch_id,

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

@@ -351,6 +351,9 @@ var WebNFCTest = (() => {
 
     setIsNDEFTech(isNdef) {
       this.is_ndef_tech_ = isNdef;
+      if (!isNdef && this.watchers_.length != 0) {
+        this.client_.onError(device.mojom.NDEFErrorType.NOT_SUPPORTED);
+      }
     }
 
     setIsFormattedTag(isFormatted) {

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

@@ -168,17 +168,27 @@ nfc_test(async (t, mockNFC) => {
 }, "NDEFRecord.toRecords returns its embedded records correctly.");
 
 nfc_test(async (t, mockNFC) => {
-  mockNFC.setIsNDEFTech(false);
+  const promises = [];
 
-  const reader = new NDEFReader();
-  reader.onreading = t.unreached_func("reading event should not be fired.");
-  await reader.scan();
+  const reader1 = new NDEFReader();
+  const readerWatcher1 = new EventWatcher(t, reader1, ["reading", "error"]);
+  const promise1 = readerWatcher1.wait_for("error").then(event => {
+    assert_true(event instanceof ErrorEvent);
+  });
+  promises.push(promise1);
+  await reader1.scan();
 
-  mockNFC.setReadingMessage(createMessage([createTextRecord(test_text_data)]));
-  await new Promise((resolve, reject) => {
-    t.step_timeout(resolve, 100);
+  const reader2 = new NDEFReader();
+  const readerWatcher2 = new EventWatcher(t, reader2, ["reading", "error"]);
+  const promise2 = readerWatcher2.wait_for("error").then(event => {
+    assert_true(event instanceof ErrorEvent);
   });
-}, "Test that NDEFReader.onreading should not be fired if the NFC tag does not \
+  promises.push(promise2);
+  await reader2.scan();
+
+  mockNFC.setIsNDEFTech(false);
+  await Promise.all(promises);
+}, "Test that NDEFReader.onerror should be fired if the NFC tag does not \
 expose NDEF technology.");
 
 nfc_test(async (t, mockNFC) => {