[WebNFC] Rename NDEFReader onerror to onreadingerror

The NDEFReader onerror event handler is renamed to onreadingerror to
make it clear to web developers that only NFC read errors will be fired
in this event as NDEFReader and NDEFWriter will soon been merged. Note
that an error message is now displayed in the devtools JS console to
help web developers to diagnose issues while it was part of the error
event before.

This CL also removes onreadingerror event fired on MojoConnectionError
as this is called if either the NFC service is unavailable in which case
NDEFReader.scan() will return a rejected promise, or when NFC permission
is revoked in which case the Permission API can be used to detect
changes.

Spec: https://github.com/w3c/web-nfc/pull/601
Bug: 520391
Change-Id: I451083fb5382149e9a1e45da31e575709f190448
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546011Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#829626}

third_party/blink/renderer/core/events/event_type_names.json5

@@ -233,6 +233,7 @@
     "quotachange",
     "ratechange",
     "reading",
+    "readingerror",
     "readystatechange",
     "reflectionchange",
     "rejectionhandled",

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

@@ -8,13 +8,12 @@
 
 #include "services/device/public/mojom/nfc.mojom-blink.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
-#include "third_party/blink/renderer/bindings/core/v8/source_location.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_ndef_scan_options.h"
 #include "third_party/blink/renderer/core/dom/abort_signal.h"
 #include "third_party/blink/renderer/core/dom/dom_exception.h"
-#include "third_party/blink/renderer/core/events/error_event.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
+#include "third_party/blink/renderer/core/inspector/console_message.h"
 #include "third_party/blink/renderer/modules/event_target_modules.h"
 #include "third_party/blink/renderer/modules/nfc/ndef_message.h"
 #include "third_party/blink/renderer/modules/nfc/ndef_reading_event.h"
@@ -198,10 +197,11 @@ void NDEFReader::OnReading(const String& serial_number,
       MakeGarbageCollected<NDEFMessage>(message)));
 }
 
-void NDEFReader::OnError(const String& message) {
-  ErrorEvent* event = ErrorEvent::Create(
-      message, SourceLocation::Capture(GetExecutionContext()), nullptr);
-  DispatchEvent(*event);
+void NDEFReader::OnReadingError(const String& message) {
+  DispatchEvent(*Event::Create(event_type_names::kReadingerror));
+  GetExecutionContext()->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+      mojom::blink::ConsoleMessageSource::kJavaScript,
+      mojom::blink::ConsoleMessageLevel::kInfo, message));
 }
 
 void NDEFReader::OnMojoConnectionError() {
@@ -212,9 +212,6 @@ void NDEFReader::OnMojoConnectionError() {
         kNotSupportedOrPermissionDenied));
     resolver_.Clear();
   }
-
-  // Dispatches an error event.
-  OnError(kNotSupportedOrPermissionDenied);
 }
 
 void NDEFReader::ContextDestroyed() {

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

@@ -41,8 +41,8 @@ class MODULES_EXPORT NDEFReader : public EventTargetWithInlineData,
   // ActiveScriptWrappable overrides.
   bool HasPendingActivity() const override;
 
-  DEFINE_ATTRIBUTE_EVENT_LISTENER(error, kError)
   DEFINE_ATTRIBUTE_EVENT_LISTENER(reading, kReading)
+  DEFINE_ATTRIBUTE_EVENT_LISTENER(readingerror, kReadingerror)
   ScriptPromise scan(ScriptState*, const NDEFScanOptions*, ExceptionState&);
 
   void Trace(Visitor*) const override;
@@ -50,7 +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(const String& message);
+  virtual void OnReadingError(const String& message);
 
   // Called by NFCProxy for notification about connection error.
   void OnMojoConnectionError();

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

@@ -12,7 +12,7 @@
 ] interface NDEFReader : EventTarget {
     [CallWith=ExecutionContext] constructor();
     attribute EventHandler onreading;
-    attribute EventHandler onerror;
+    attribute EventHandler onreadingerror;
 
     [CallWith=ScriptState, RaisesException] Promise<void> scan(
         optional NDEFScanOptions options={});

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

@@ -115,11 +115,11 @@ void NFCProxy::OnWatch(const Vector<uint32_t>& watch_ids,
 
 void NFCProxy::OnError(device::mojom::blink::NDEFErrorPtr 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.
+  // because a reader's onreadingerror event handler may remove itself from
+  // |readers_| just during the iteration process.
   ReaderMap copy = readers_;
   for (auto& pair : copy) {
-    pair.key->OnError(error->error_message);
+    pair.key->OnReadingError(error->error_message);
   }
 }
 

third_party/blink/web_tests/NeverFixTests

@@ -1855,6 +1855,8 @@ external/wpt/visual-viewport/viewport-scale-manual.html [ Skip ]
 external/wpt/visual-viewport/viewport-scroll-event-manual.html [ Skip ]
 external/wpt/visual-viewport/viewport-url-bar-changes-height-manual.html [ Skip ]
 external/wpt/screen-wake-lock/wakelock-document-hidden-manual.https.html [ Skip ]
+external/wpt/web-nfc/NDEFReader-document-hidden-manual.https.html [ Skip ]
+external/wpt/web-nfc/NDEFWriter-document-hidden-manual.https.html [ Skip ]
 external/wpt/web-share/share-cancel-manual.https.html [ Skip ]
 external/wpt/web-share/share-extra-argument-manual.https.html [ Skip ]
 external/wpt/web-share/share-extra-field-manual.https.html [ Skip ]

third_party/blink/web_tests/external/wpt/interfaces/web-nfc.idl

@@ -55,8 +55,8 @@ interface NDEFWriter {
 interface NDEFReader : EventTarget {
   constructor();
 
-  attribute EventHandler onerror;
   attribute EventHandler onreading;
+  attribute EventHandler onreadingerror;
 
   Promise<undefined> scan(optional NDEFScanOptions options={});
 };

third_party/blink/web_tests/external/wpt/web-nfc/NDEFReader-document-hidden-manual.https-expected.txt

-This is a testharness.js-based test.
-FAIL Test NDEFReader.onreading is not fired when document is hidden assert_equals: Expected reading event, but got error event instead expected "reading" but got "error"
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/web-nfc/NDEFReader-document-hidden-manual.https.html

@@ -9,12 +9,13 @@
 <script>
 
 nfc_test(async (t, mockNFC) => {
-  mockNFC.simulateClosedPipe();
   const reader = new NDEFReader();
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
-  const promise = readerWatcher.wait_for("reading").then(event => {
-    if (document.hidden) reject();
-    resolve();
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
+  const promise = new Promise((resolve, reject) => {
+    readerWatcher.wait_for("reading").then(event => {
+      if (document.hidden) reject();
+      else resolve();
+    });
   });
   await reader.scan();
   await promise;

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

@@ -46,14 +46,8 @@ nfc_test(async t => {
 nfc_test(async (t, mockNFC) => {
   mockNFC.simulateClosedPipe();
   const reader = new NDEFReader();
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
-  const promise = readerWatcher.wait_for("error").then(event => {
-    assert_true(event instanceof ErrorEvent);
-  });
   await promise_rejects_dom(t, 'NotSupportedError', reader.scan());
-  await promise;
-}, "Test that an error event happens if no implementation for NFC Mojo interface \
-is available.");
+}, "NDEFReader.scan should faild if no implementation for NFC Mojo interface.");
 
 nfc_test(async (t, mockNFC) => {
   mockNFC.setHWStatus(NFCHWStatus.DISABLED);
@@ -70,7 +64,7 @@ nfc_test(async (t, mockNFC) => {
 nfc_test(async (t, mockNFC) => {
   const reader = new NDEFReader();
   const controller = new AbortController();
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
   const promise = readerWatcher.wait_for("reading").then(event => {
     assert_true(event instanceof NDEFReadingEvent);
     controller.abort();
@@ -84,7 +78,7 @@ nfc_test(async (t, mockNFC) => {
 nfc_test(async (t, mockNFC) => {
   const reader = new NDEFReader();
   const controller = new AbortController();
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
   const promise = readerWatcher.wait_for("reading").then(event => {
     assert_true(event instanceof NDEFReadingEvent);
     controller.abort();
@@ -114,7 +108,7 @@ the scan invocation.");
 nfc_test(async (t, mockNFC) => {
   const reader = new NDEFReader();
   const controller = new AbortController();
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
   const message = createMessage([createTextRecord(test_text_data)]);
   const promise = readerWatcher.wait_for("reading").then(event => {
     assert_true(event instanceof NDEFReadingEvent);
@@ -135,7 +129,7 @@ nfc_test(async (t, mockNFC) => {
 nfc_test(async (t, mockNFC) => {
   const reader = new NDEFReader();
   const controller = new AbortController();
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
   const promise = readerWatcher.wait_for("reading").then(event => {
     controller.abort();
     assert_true(event instanceof NDEFReadingEvent);
@@ -172,7 +166,7 @@ nfc_test(async (t, mockNFC) => {
 nfc_test(async (t, mockNFC) => {
   const reader = new NDEFReader();
   const controller = new AbortController();
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
   const promise = readerWatcher.wait_for("reading").then(event => {
     controller.abort();
     assert_true(event instanceof NDEFReadingEvent);
@@ -225,30 +219,26 @@ nfc_test(async (t, mockNFC) => {
   const promises = [];
 
   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);
-  });
+  const readerWatcher1 = new EventWatcher(t, reader1, ["reading", "readingerror"]);
+  const promise1 = readerWatcher1.wait_for("readingerror");
   promises.push(promise1);
   await reader1.scan();
 
   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);
-  });
+  const readerWatcher2 = new EventWatcher(t, reader2, ["reading", "readingerror"]);
+  const promise2 = readerWatcher2.wait_for("readingerror");
   promises.push(promise2);
   await reader2.scan();
 
   mockNFC.simulateNonNDEFTagDiscovered();
   await Promise.all(promises);
-}, "Test that NDEFReader.onerror should be fired if the NFC tag does not \
+}, "Test that NDEFReader.onreadingerror should be fired if the NFC tag does not \
 expose NDEF technology.");
 
 nfc_test(async (t, mockNFC) => {
   const reader = new NDEFReader();
   const controller = new AbortController();
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
   const promise = readerWatcher.wait_for("reading").then(event => {
     assert_equals(event.serialNumber, fake_tag_serial_number);
     assert_equals(event.message.records.length, 0);
@@ -272,7 +262,7 @@ nfc_test(async (t, mockNFC) => {
                                 createUrlRecord(test_url_data, true),
                                 createRecord('w3.org:xyz', test_buffer_data)],
                                 test_message_origin);
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
   const promise = readerWatcher.wait_for("reading").then(event => {
     assert_equals(event.serialNumber, fake_tag_serial_number);
     assertWebNDEFMessagesEqual(event.message, new NDEFMessage(message));

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

@@ -11,7 +11,7 @@
 nfc_test(async (t, mockNFC) => {
   const reader = new NDEFReader();
   const controller = new AbortController();
-  const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+  const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
 
   const promise = readerWatcher.wait_for("reading").then(event => {
     assert_true(event instanceof NDEFReadingEvent);

third_party/blink/web_tests/external/wpt/web-nfc/NDEFWriter-document-hidden-manual.https-expected.txt

-This is a testharness.js-based test.
-FAIL Test NDEFWriter.write operation should be suspend when document is not visible promise_test: Unhandled rejection with value: undefined
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/web-nfc/resources/nfc-helpers.js

@@ -204,7 +204,7 @@ function testMultiScanOptions(message, scanOptions, unmatchedScanOptions, desc)
     unmatchedScanOptions.signal = controller.signal;
     await reader1.scan(unmatchedScanOptions);
 
-    const readerWatcher = new EventWatcher(t, reader2, ["reading", "error"]);
+    const readerWatcher = new EventWatcher(t, reader2, ["reading", "readingerror"]);
     const promise = readerWatcher.wait_for("reading").then(event => {
       controller.abort();
       assertWebNDEFMessagesEqual(event.message, new NDEFMessage(message));
@@ -221,7 +221,7 @@ function testMultiMessages(message, scanOptions, unmatchedMessage, desc) {
   nfc_test(async (t, mockNFC) => {
     const reader = new NDEFReader();
     const controller = new AbortController();
-    const readerWatcher = new EventWatcher(t, reader, ["reading", "error"]);
+    const readerWatcher = new EventWatcher(t, reader, ["reading", "readingerror"]);
     const promise = readerWatcher.wait_for("reading").then(event => {
       controller.abort();
       assertWebNDEFMessagesEqual(event.message, new NDEFMessage(message));

third_party/blink/web_tests/http/tests/origin_trials/webexposed/web-nfc-origin-trial-interfaces.html

@@ -13,7 +13,7 @@ test(t => {
   OriginTrialsHelper.check_properties_exist(this,
       {
         'NDEFMessage': ['records'],
-        'NDEFReader': ['scan', 'onreading', 'onerror'],
+        'NDEFReader': ['scan', 'onreading', 'onreadingerror'],
         'NDEFReadingEvent': ['serialNumber', 'message'],
         'NDEFRecord': ['recordType', 'mediaType', 'id', 'encoding', 'lang', 'data', 'toRecords'],
         'NDEFWriter': ['write'],

third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt

@@ -5377,12 +5377,12 @@ interface NDEFMessage
     method constructor
 interface NDEFReader : EventTarget
     attribute @@toStringTag
-    getter onerror
     getter onreading
+    getter onreadingerror
     method constructor
     method scan
-    setter onerror
     setter onreading
+    setter onreadingerror
 interface NDEFReadingEvent : Event
     attribute @@toStringTag
     getter message