Barcode/Face detection: reject requests in platforms where not implemented

Barcode/Face detection: reject requests in platforms where not implemented

This CL adds on_connection_error listeners to the mojo interface
pointers used for face/barcode detection. The connection error
handler is used to reject all Promises related to the given interface,
otherwise requests from Blink are neither resolved nor rejected (see bug).

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/62beea5d8166bee43361033475448a0a08cd5bfa
Cr-Commit-Position: refs/heads/master@{#435484}

[mcasas, 2016-11-30 02:16:12]

Description was changed from

==========
Barcode/Face detection: add dummy intf impl rejecting requests in platforms
where not implemented

BUG=668343
==========

to

==========
Barcode/Face detection: add dummy intf impl rejecting requests in platforms
where not implemented

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[mcasas, 2016-11-30 03:01:25]

Patchset #2 (id:20001) has been deleted

[mcasas, 2016-11-30 04:56:06]

Patchset #1 (id:1) has been deleted

[mcasas, 2016-11-30 04:56:18]

The CQ bit was checked by mcasas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 04:56:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 05:03:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 05:03:08]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[mcasas, 2016-11-30 16:21:58]

Description was changed from

==========
Barcode/Face detection: add dummy intf impl rejecting requests in platforms
where not implemented

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Barcode/Face detection: add interface impl rejecting requests in platforms where
not implemented

This CL adds a dummy {Barcode/Face}DetectionImpl in browser to be 
used in platforms where there is no meaningful implementation, 
otherwise requests from Blink are neither resolved nor rejected (see bug).

To convey the meaning of "not implemented", a new optional
field is added to the mojo response. Since mojo cannot make
primitive types optional, the enum DetectionErrorType is wrapped 
in a mojo Struct (similar to what nfc.mojom does [1]).


[1]
https://cs.chromium.org/chromium/src/device/nfc/nfc.mojom?q=nfc.mojom&sq=pack...

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[mcasas, 2016-11-30 16:22:28]

mcasas@chromium.org changed reviewers:
+ reillyg@chromium.org

[mcasas, 2016-11-30 16:22:29]

reillyg@ PTAL

[Reilly Grant (use Gerrit), 2016-11-30 20:27:04]

As discussed OOB mcasas@ will investigate adding a connection error handler to
the Blink side to detect when the pipe is closed to report the "not supported"
error.

[mcasas, 2016-11-30 21:20:40]

Description was changed from

==========
Barcode/Face detection: add interface impl rejecting requests in platforms where
not implemented

This CL adds a dummy {Barcode/Face}DetectionImpl in browser to be 
used in platforms where there is no meaningful implementation, 
otherwise requests from Blink are neither resolved nor rejected (see bug).

To convey the meaning of "not implemented", a new optional
field is added to the mojo response. Since mojo cannot make
primitive types optional, the enum DetectionErrorType is wrapped 
in a mojo Struct (similar to what nfc.mojom does [1]).


[1]
https://cs.chromium.org/chromium/src/device/nfc/nfc.mojom?q=nfc.mojom&sq=pack...

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Barcode/Face detection: reject requests in platforms where not implemented

This CL adds on_connection_error listeners to the mojo interface
pointers used for face/barcode detection. The connection error
handler is used to reject all Promises related to the given interface,
otherwise requests from Blink are neither resolved nor rejected (see bug).

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[mcasas, 2016-11-30 21:33:20]

reillyg@ PTAL

[Reilly Grant (use Gerrit), 2016-11-30 21:43:28]

https://codereview.chromium.org/2538053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp (right):

https://codereview.chromium.org/2538053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp:174:
DCHECK(m_faceService);
We should still have the code to reject the promise here.

https://codereview.chromium.org/2538053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp:373:
m_faceServiceRequests.clear();
Reset m_faceService so that code above knows the connection has be closed. You
only get one connection error.

[mcasas, 2016-11-30 21:49:51]

ptal

https://codereview.chromium.org/2538053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp (right):

https://codereview.chromium.org/2538053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp:174:
DCHECK(m_faceService);
On 2016/11/30 21:43:28, Reilly Grant wrote:
> We should still have the code to reject the promise here.

Done.

https://codereview.chromium.org/2538053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp:373:
m_faceServiceRequests.clear();
On 2016/11/30 21:43:28, Reilly Grant wrote:
> Reset m_faceService so that code above knows the connection has be closed. You
> only get one connection error.

Done.

[Reilly Grant (use Gerrit), 2016-11-30 21:58:47]

https://codereview.chromium.org/2538053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp (right):

https://codereview.chromium.org/2538053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp:351: if
(!m_faceServiceRequests.contains(resolver))
I think this can be a DCHECK. The reason we have this check in WebUSB is because
the request sets get cleared when the ExecutionContext is destroyed. I don't
think that's actually really necessary anymore.

https://codereview.chromium.org/2538053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp:368: if
(!m_barcodeServiceRequests.contains(resolver))
Ditto.

[mcasas, 2016-11-30 22:03:16]

https://codereview.chromium.org/2538053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp (right):

https://codereview.chromium.org/2538053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp:351: if
(!m_faceServiceRequests.contains(resolver))
On 2016/11/30 21:58:47, Reilly Grant wrote:
> I think this can be a DCHECK. The reason we have this check in WebUSB is
because
> the request sets get cleared when the ExecutionContext is destroyed. I don't
> think that's actually really necessary anymore.

Done.

https://codereview.chromium.org/2538053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/ShapeDetector.cpp:368: if
(!m_barcodeServiceRequests.contains(resolver))
On 2016/11/30 21:58:47, Reilly Grant wrote:
> Ditto.

Done.

[Reilly Grant (use Gerrit), 2016-11-30 22:10:07]

lgtm

[mcasas, 2016-11-30 22:18:30]

mcasas@chromium.org changed reviewers:
+ haraken@chromium.org

[mcasas, 2016-11-30 22:18:30]

haraken@ RS plz.

[mcasas, 2016-11-30 22:24:13]

The CQ bit was checked by mcasas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 22:24:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[haraken, 2016-11-30 23:33:56]

LGTM

[mcasas, 2016-11-30 23:35:59]

The CQ bit was unchecked by mcasas@chromium.org

[mcasas, 2016-11-30 23:36:19]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2016-11-30 23:36:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 23:59:50]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1480548979029080,
"parent_rev": "9a7d0bddc71964fa8d0adbffbc5d5a8d9a0ea6ab", "commit_rev":
"95e5e78ce4f17f8dc15a193557a84997357a5bc7"}

[commit-bot: I haz the power, 2016-12-01 00:00:19]

Description was changed from

==========
Barcode/Face detection: reject requests in platforms where not implemented

This CL adds on_connection_error listeners to the mojo interface
pointers used for face/barcode detection. The connection error
handler is used to reject all Promises related to the given interface,
otherwise requests from Blink are neither resolved nor rejected (see bug).

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Barcode/Face detection: reject requests in platforms where not implemented

This CL adds on_connection_error listeners to the mojo interface
pointers used for face/barcode detection. The connection error
handler is used to reject all Promises related to the given interface,
otherwise requests from Blink are neither resolved nor rejected (see bug).

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-12-01 00:00:20]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-12-01 00:05:03]

Description was changed from

==========
Barcode/Face detection: reject requests in platforms where not implemented

This CL adds on_connection_error listeners to the mojo interface
pointers used for face/barcode detection. The connection error
handler is used to reject all Promises related to the given interface,
otherwise requests from Blink are neither resolved nor rejected (see bug).

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Barcode/Face detection: reject requests in platforms where not implemented

This CL adds on_connection_error listeners to the mojo interface
pointers used for face/barcode detection. The connection error
handler is used to reject all Promises related to the given interface,
otherwise requests from Blink are neither resolved nor rejected (see bug).

BUG=668343
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/62beea5d8166bee43361033475448a0a08cd5bfa
Cr-Commit-Position: refs/heads/master@{#435484}
==========

[commit-bot: I haz the power, 2016-12-01 00:05:04]

Patchset 4 (id:??) landed as
https://crrev.com/62beea5d8166bee43361033475448a0a08cd5bfa
Cr-Commit-Position: refs/heads/master@{#435484}