From 33ca5fad2af9cb4d0b97a31d017d403bd2d38f10 Mon Sep 17 00:00:00 2001
From: Francois Beaufort <beaufort.francois@gmail.com>
Date: Fri, 20 Nov 2020 12:19:04 +0000
Subject: [PATCH] [WebNFC] Rename NDEFReader onerror to onreadingerror
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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/+/2546011
Reviewed-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}
GitOrigin-RevId: 5d80d77e62168984affeee6ac9e335a788a997df
---
 .../core/events/event_type_names.json5        |  1 +
 blink/renderer/modules/nfc/ndef_reader.cc     | 15 ++++----
 blink/renderer/modules/nfc/ndef_reader.h      |  4 +--
 blink/renderer/modules/nfc/ndef_reader.idl    |  2 +-
 blink/renderer/modules/nfc/nfc_proxy.cc       |  6 ++--
 blink/web_tests/NeverFixTests                 |  2 ++
 .../external/wpt/interfaces/web-nfc.idl       |  2 +-
 ...-document-hidden-manual.https-expected.txt |  4 ---
 ...EFReader-document-hidden-manual.https.html | 11 +++---
 .../wpt/web-nfc/NDEFReader_scan.https.html    | 36 +++++++------------
 .../web-nfc/NDEFReader_scan_iframe.https.html |  2 +-
 ...-document-hidden-manual.https-expected.txt |  4 ---
 .../wpt/web-nfc/resources/nfc-helpers.js      |  4 +--
 .../web-nfc-origin-trial-interfaces.html      |  2 +-
 .../global-interface-listing-expected.txt     |  4 +--
 15 files changed, 41 insertions(+), 58 deletions(-)
 delete mode 100644 blink/web_tests/external/wpt/web-nfc/NDEFReader-document-hidden-manual.https-expected.txt
 delete mode 100644 blink/web_tests/external/wpt/web-nfc/NDEFWriter-document-hidden-manual.https-expected.txt

diff --git a/blink/renderer/core/events/event_type_names.json5 b/blink/renderer/core/events/event_type_names.json5
index ce539efd0b9e..ef4697ad8070 100644
--- a/blink/renderer/core/events/event_type_names.json5
+++ b/blink/renderer/core/events/event_type_names.json5
@@ -233,6 +233,7 @@
     "quotachange",
     "ratechange",
     "reading",
+    "readingerror",
     "readystatechange",
     "reflectionchange",
     "rejectionhandled",
diff --git a/blink/renderer/modules/nfc/ndef_reader.cc b/blink/renderer/modules/nfc/ndef_reader.cc
index 82d73c042325..da91ed882a34 100644
--- a/blink/renderer/modules/nfc/ndef_reader.cc
+++ b/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() {
diff --git a/blink/renderer/modules/nfc/ndef_reader.h b/blink/renderer/modules/nfc/ndef_reader.h
index ee1a93d9d4db..cf3edc5126f5 100644
--- a/blink/renderer/modules/nfc/ndef_reader.h
+++ b/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();
diff --git a/blink/renderer/modules/nfc/ndef_reader.idl b/blink/renderer/modules/nfc/ndef_reader.idl
index 280ebf5ac6ec..2359a7a20701 100644
--- a/blink/renderer/modules/nfc/ndef_reader.idl
+++ b/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={});
diff --git a/blink/renderer/modules/nfc/nfc_proxy.cc b/blink/renderer/modules/nfc/nfc_proxy.cc
index 2cf4c852dc3e..fb380ee6b564 100644
--- a/blink/renderer/modules/nfc/nfc_proxy.cc
+++ b/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);
   }
 }
 
diff --git a/blink/web_tests/NeverFixTests b/blink/web_tests/NeverFixTests
index ea8e9cb03f00..71cc2afec0dc 100644
--- a/blink/web_tests/NeverFixTests
+++ b/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 ]
diff --git a/blink/web_tests/external/wpt/interfaces/web-nfc.idl b/blink/web_tests/external/wpt/interfaces/web-nfc.idl
index 14df65c6cecc..bdf0324d96d6 100644
--- a/blink/web_tests/external/wpt/interfaces/web-nfc.idl
+++ b/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={});
 };
diff --git a/blink/web_tests/external/wpt/web-nfc/NDEFReader-document-hidden-manual.https-expected.txt b/blink/web_tests/external/wpt/web-nfc/NDEFReader-document-hidden-manual.https-expected.txt
deleted file mode 100644
index e2f237773678..000000000000
--- a/blink/web_tests/external/wpt/web-nfc/NDEFReader-document-hidden-manual.https-expected.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-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.
-
diff --git a/blink/web_tests/external/wpt/web-nfc/NDEFReader-document-hidden-manual.https.html b/blink/web_tests/external/wpt/web-nfc/NDEFReader-document-hidden-manual.https.html
index f2027dc99d1d..ffbd7ebc4561 100644
--- a/blink/web_tests/external/wpt/web-nfc/NDEFReader-document-hidden-manual.https.html
+++ b/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;
diff --git a/blink/web_tests/external/wpt/web-nfc/NDEFReader_scan.https.html b/blink/web_tests/external/wpt/web-nfc/NDEFReader_scan.https.html
index 46f2871f3dc9..b551af0150f0 100644
--- a/blink/web_tests/external/wpt/web-nfc/NDEFReader_scan.https.html
+++ b/blink/web_tests/external/wpt/web-nfc/NDEFReader_scan.https.html
@@ -46,14 +46,8 @@
 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) => {
   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) => {
   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 @@
 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) => {
   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) => {
   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 @@
   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 @@
                                 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));
diff --git a/blink/web_tests/external/wpt/web-nfc/NDEFReader_scan_iframe.https.html b/blink/web_tests/external/wpt/web-nfc/NDEFReader_scan_iframe.https.html
index 9bdb07136821..8803c25fb0a5 100644
--- a/blink/web_tests/external/wpt/web-nfc/NDEFReader_scan_iframe.https.html
+++ b/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);
diff --git a/blink/web_tests/external/wpt/web-nfc/NDEFWriter-document-hidden-manual.https-expected.txt b/blink/web_tests/external/wpt/web-nfc/NDEFWriter-document-hidden-manual.https-expected.txt
deleted file mode 100644
index d700bd80786f..000000000000
--- a/blink/web_tests/external/wpt/web-nfc/NDEFWriter-document-hidden-manual.https-expected.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-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.
-
diff --git a/blink/web_tests/external/wpt/web-nfc/resources/nfc-helpers.js b/blink/web_tests/external/wpt/web-nfc/resources/nfc-helpers.js
index 13cbe49370e9..0b26d09c96aa 100644
--- a/blink/web_tests/external/wpt/web-nfc/resources/nfc-helpers.js
+++ b/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));
diff --git a/blink/web_tests/http/tests/origin_trials/webexposed/web-nfc-origin-trial-interfaces.html b/blink/web_tests/http/tests/origin_trials/webexposed/web-nfc-origin-trial-interfaces.html
index c6d3474c9d5a..0493668f0fff 100644
--- a/blink/web_tests/http/tests/origin_trials/webexposed/web-nfc-origin-trial-interfaces.html
+++ b/blink/web_tests/http/tests/origin_trials/webexposed/web-nfc-origin-trial-interfaces.html
@@ -13,7 +13,7 @@
   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'],
diff --git a/blink/web_tests/webexposed/global-interface-listing-expected.txt b/blink/web_tests/webexposed/global-interface-listing-expected.txt
index b9604b7e305f..90c458f9e086 100644
--- a/blink/web_tests/webexposed/global-interface-listing-expected.txt
+++ b/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