1733174 - [wpt-sync] Sync PR 30896 - nfc: Limit amount NDEFMessage objects that can be nested.

Status: RESOLVED FIXED

(Testing :: web-platform-tests, task, P4)

Description

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Sync web-platform-tests PR 30896 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/30896
Details from upstream follow.

Raphael Kubo da Costa <[e-mail]> wrote:

nfc: Limit amount NDEFMessage objects that can be nested.

Implement https://github.com/w3c/web-nfc/pull/621, which adds a
hard-coded maximum of 32 NDEFMessage objects that can be in the same
chain.

https://github.com/w3c/web-nfc/pull/454 (and
https://chromium-review.googlesource.com/c/chromium/src/+/1941083 on the
implementation side) ended up creating a cycle between
NDEFRecordInit.data and NDEFMessageInit, as the former can be an
NDEFMessage, which could have one or more entries pointing to the same
NDEFRecordInit. Recursion in Web IDL dictionaries are disallowed, so
this change also fixes some invalid IDL that was present in the spec and
our IDL file.

This CL attempts to optimize or modify other parts of the code as little
as possible, but the the change ended up being a bit big mainly due to
two things:

• Since NDEFRecordInit.data's type is now any, we need to do more type
  conversions ourselves in NDEFRecord that we used to get for free when
  using IDL unions. This means attempting to convert a V8 value into one
  or more C++ types and accounting for conversion failures.
• In order to do the above, we need access to v8::Isolate, so several
  functions in NDEFRecord that used to take an ExecutionContext needed
  to be changed to take a ScriptState instead.

The usage of ScriptState and the need to perform multiple type
conversions also caused NDEFReadingEvent's constructor to start
passing one to NDEFMessage, which at the end causes the NDEFRecords
that may get created to contain a proper lang attribute rather than
null. This is a user-visible change, but I could not find anything in
the spec supporting the previous behavior.

Bug: 1242274
Change-Id: Ia4de230ea45f63f903760865bf85594f79da9eb7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3169656
Reviewed-by: Yuki Shiino \<[e-mail]>
Reviewed-by: François Beaufort \<[e-mail]>
Reviewed-by: Reilly Grant \<[e-mail]>
Commit-Queue: Raphael Kubo da Costa \<[e-mail]>
Cr-Commit-Position: refs/heads/main@{#925263}

Comment 1

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

PR 30896 applied with additional changes from upstream: a6760fe8d61bb9877c4347e1050787eed7fa7b1f

Comment 2

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Pushed to try (stability) https://treeherder.mozilla.org/#/jobs?repo=try&revision=b29110e4ae0e35a41257af56f4cfaf64fa57ccac

Comment 3

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

CI Results

Ran 11 Firefox configurations based on mozilla-central, and Firefox, Chrome, and Safari on GitHub CI

Total 2 tests and 6 subtests

Status Summary

Firefox

OK : 2
FAIL: 11

Chrome

OK : 2
FAIL: 11

Safari

OK : 2
FAIL: 11

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Details

New Tests That Don't Pass

/web-nfc/NDEFMessage_recursion-limit.https.window.html: OK [GitHub], SKIP [Gecko-android-em-7.0-x86_64-lite-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-lite-qr-opt-geckoview, Gecko-android-em-7.0-x86_64-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-qr-opt-geckoview, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-linux1804-64-tsan-qr-opt, Gecko-windows10-32-2004-qr-debug, Gecko-windows10-32-2004-qr-opt, Gecko-windows10-64-2004-qr-debug, Gecko-windows10-64-2004-qr-opt] (Chrome: OK, Safari: OK)
NDEFRecord and NDEFMessage cycle in external records: FAIL (Chrome: FAIL, Safari: FAIL)
NDEFRecord and NDEFMessage cycle in local records: FAIL (Chrome: FAIL, Safari: FAIL)
NDEFRecord and NDEFMessage cycle in smart poster records: FAIL (Chrome: FAIL, Safari: FAIL)
Create too many nested NDEFMessages: FAIL (Chrome: FAIL, Safari: FAIL)
Nest maximum number of NDEFMessages: FAIL (Chrome: FAIL, Safari: FAIL)
/web-nfc/NDEFReadingEvent_constructor.https.window.html: OK [GitHub], SKIP [Gecko-android-em-7.0-x86_64-lite-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-lite-qr-opt-geckoview, Gecko-android-em-7.0-x86_64-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-qr-opt-geckoview, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-linux1804-64-tsan-qr-opt, Gecko-windows10-32-2004-qr-debug, Gecko-windows10-32-2004-qr-opt, Gecko-windows10-64-2004-qr-debug, Gecko-windows10-64-2004-qr-opt] (Chrome: OK, Safari: OK)
NDEFReadingEvent constructor without init dict: FAIL (Chrome: FAIL, Safari: FAIL)
NDEFReadingEvent constructor failed to construct its NDEFMessage: FAIL (Chrome: FAIL, Safari: FAIL)
NDEFReadingEvent constructor with null serialNumber: FAIL (Chrome: FAIL, Safari: FAIL)
NDEFReadingEvent constructor with serialNumber not present: FAIL (Chrome: FAIL, Safari: FAIL)
NDEFReadingEvent constructor with valid parameters: FAIL (Chrome: FAIL, Safari: FAIL)
NDEFReadingEvent constructor sets NDEFRecord#lang for the text records it embeds: FAIL (Chrome: FAIL, Safari: FAIL)

Tests Disabled in Gecko Infrastructure

/web-nfc/NDEFMessage_recursion-limit.https.window.html: OK [GitHub], SKIP [Gecko-android-em-7.0-x86_64-lite-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-lite-qr-opt-geckoview, Gecko-android-em-7.0-x86_64-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-qr-opt-geckoview, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-linux1804-64-tsan-qr-opt, Gecko-windows10-32-2004-qr-debug, Gecko-windows10-32-2004-qr-opt, Gecko-windows10-64-2004-qr-debug, Gecko-windows10-64-2004-qr-opt] (Chrome: OK, Safari: OK)
/web-nfc/NDEFReadingEvent_constructor.https.window.html: OK [GitHub], SKIP [Gecko-android-em-7.0-x86_64-lite-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-lite-qr-opt-geckoview, Gecko-android-em-7.0-x86_64-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-qr-opt-geckoview, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-linux1804-64-tsan-qr-opt, Gecko-windows10-32-2004-qr-debug, Gecko-windows10-32-2004-qr-opt, Gecko-windows10-64-2004-qr-debug, Gecko-windows10-64-2004-qr-opt] (Chrome: OK, Safari: OK)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/872a1784ca54
[wpt PR 30896] - nfc: Limit amount NDEFMessage objects that can be nested., a=testonly

Comment 5

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/872a1784ca54