Re: [web-nfc] Fix #592: allow multiple readers but no multiple scans on any reader (#611)

We had a call with Arno and Riju, and that clarified things. I discovered a non-related bug in the scan algo, fixed in the commit above.

The `has_pending_scan_request_` flag indeed covers the time between the `scan()` invocation and its promise resolving, i.e. until the `reader` is added to the activated reader objects.
Arno tried removing the flag and resulted in the renderer crashing at a second invocation of `scan()` because it ran into the `DCHECK()` in the beginning of `NfcProxy::StartReading()`. Which IMHO is an implementation bug, since it was supposed to fail the promise on that condition, not to crash the renderer.  The `has_pending_scan_request_` seems to be a _workaround_ for that bug, not an algorithmic necessity.

In the scan algorithm in the spec is IMHO correct: the condition on `reader` being in the activated reader list is sufficient since the reading algorithm is triggered on the same condition. Single point of exclusion is desired, so the algorithm is correct. We don't need to factor out a separate internal slot to reflect the current implementation behavior that uses `has_pending_scan_request_`.

The existing WPT should be working as defined with the current specification of the scan algorithm.

-- 
GitHub Notification of comment by zolkis
Please view or discuss this issue at https://github.com/w3c/web-nfc/pull/611#issuecomment-750321508 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Wednesday, 23 December 2020 14:22:25 UTC