Shape Detection: add idl and mojom for face landmarks and wire for Mac

Shape Detection: add idl and mojom for face landmarks and wire for Mac

Spec issue [1] and associated commit [2] added landmarks to the
detected faces, in particular eyes and mouth.  This CL:

-  adds the appropriate idls and mojo definitions to support those;

- adapts facedetection mojom Detect() to return an array<structs> 
 ISO a struct{ array<> }, causing a few changes throughout, including
 on Android impl (which doesn't support landmark detection);

- wires detection of said landmarks to Mac Face Detection impl and
 updates the associated unittests.



[1] https://github.com/WICG/shape-detection-api/issues/7
[2] https://github.com/WICG/shape-detection-api/commit/dd90bf57313ea3d76b04c1b1c33d44fd5df2d67e

BUG=718693
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2859413002
Cr-Commit-Position: refs/heads/master@{#469820}
Committed: https://chromium.googlesource.com/chromium/src/+/04f35409eaca0779dbe9b8435f20901cf510440b

[mcasas, 2017-05-05 02:32:36]

Description was changed from

==========
Shape Detection: add Face landmarks, wire for Mac

stylez


blind update java


blind update java


Wired the eyes/mouth to Mac Face Detection impl, seems to be working


Adapted facedetection.mojom to return an array<structs> ISO a struct{ array<> },
throughout


Added proto Landmark.idl and its use in DetectedFace

BUG=
==========

to

==========
Shape Detection: add Face landmarks, wire for Mac

stylez


blind update java


blind update java


Wired the eyes/mouth to Mac Face Detection impl, seems to be working


Adapted facedetection.mojom to return an array<structs> ISO a struct{ array<> },
throughout


Added proto Landmark.idl and its use in DetectedFace

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[mcasas, 2017-05-05 02:36:15]

Description was changed from

==========
Shape Detection: add Face landmarks, wire for Mac

stylez


blind update java


blind update java


Wired the eyes/mouth to Mac Face Detection impl, seems to be working


Adapted facedetection.mojom to return an array<structs> ISO a struct{ array<> },
throughout


Added proto Landmark.idl and its use in DetectedFace

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Shape Detection: add Face landmarks, wire for Mac

Wired the eyes/mouth to Mac Face Detection impl, seems to be working

Adapted facedetection.mojom to return an array<structs> ISO a struct{ array<> },
throughout

Added proto Landmark.idl and its use in DetectedFace

BUG=

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[mcasas, 2017-05-05 02:55:39]

Patchset #1 (id:1) has been deleted

[mcasas, 2017-05-05 02:55:45]

Patchset #1 (id:20001) has been deleted

[mcasas, 2017-05-05 04:31:52]

Description was changed from

==========
Shape Detection: add Face landmarks, wire for Mac

Wired the eyes/mouth to Mac Face Detection impl, seems to be working

Adapted facedetection.mojom to return an array<structs> ISO a struct{ array<> },
throughout

Added proto Landmark.idl and its use in DetectedFace

BUG=

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Shape Detection: add idl and mojom for face landmarks and wire for Mac

Spec issue [1] and associated commit [2] added landmarks to the
detected faces, in particular eyes and mouth.  This CL:

-  adds the appropriate idls and mojo definitions to support those;

- adapts facedetection mojom Detect() to return an array<structs> 
 ISO a struct{ array<> }, causing a few changes throughout, including
 on Android impl (which doesn't support landmark detection);

- wired detection of said landmarks to Mac Face Detection impl and
 updates the associated unittests.



[1] https://github.com/WICG/shape-detection-api/issues/7
[2]
https://github.com/WICG/shape-detection-api/commit/dd90bf57313ea3d76b04c1b1c3...

BUG=

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[mcasas, 2017-05-05 04:36:42]

Description was changed from

==========
Shape Detection: add idl and mojom for face landmarks and wire for Mac

Spec issue [1] and associated commit [2] added landmarks to the
detected faces, in particular eyes and mouth.  This CL:

-  adds the appropriate idls and mojo definitions to support those;

- adapts facedetection mojom Detect() to return an array<structs> 
 ISO a struct{ array<> }, causing a few changes throughout, including
 on Android impl (which doesn't support landmark detection);

- wired detection of said landmarks to Mac Face Detection impl and
 updates the associated unittests.



[1] https://github.com/WICG/shape-detection-api/issues/7
[2]
https://github.com/WICG/shape-detection-api/commit/dd90bf57313ea3d76b04c1b1c3...

BUG=

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Shape Detection: add idl and mojom for face landmarks and wire for Mac

Spec issue [1] and associated commit [2] added landmarks to the
detected faces, in particular eyes and mouth.  This CL:

-  adds the appropriate idls and mojo definitions to support those;

- adapts facedetection mojom Detect() to return an array<structs> 
 ISO a struct{ array<> }, causing a few changes throughout, including
 on Android impl (which doesn't support landmark detection);

- wired detection of said landmarks to Mac Face Detection impl and
 updates the associated unittests.



[1] https://github.com/WICG/shape-detection-api/issues/7
[2]
https://github.com/WICG/shape-detection-api/commit/dd90bf57313ea3d76b04c1b1c3...

BUG=718693

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[mcasas, 2017-05-05 04:36:50]

Patchset #1 (id:40001) has been deleted

[mcasas, 2017-05-05 04:43:34]

Description was changed from

==========
Shape Detection: add idl and mojom for face landmarks and wire for Mac

Spec issue [1] and associated commit [2] added landmarks to the
detected faces, in particular eyes and mouth.  This CL:

-  adds the appropriate idls and mojo definitions to support those;

- adapts facedetection mojom Detect() to return an array<structs> 
 ISO a struct{ array<> }, causing a few changes throughout, including
 on Android impl (which doesn't support landmark detection);

- wired detection of said landmarks to Mac Face Detection impl and
 updates the associated unittests.



[1] https://github.com/WICG/shape-detection-api/issues/7
[2]
https://github.com/WICG/shape-detection-api/commit/dd90bf57313ea3d76b04c1b1c3...

BUG=718693

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Shape Detection: add idl and mojom for face landmarks and wire for Mac

Spec issue [1] and associated commit [2] added landmarks to the
detected faces, in particular eyes and mouth.  This CL:

-  adds the appropriate idls and mojo definitions to support those;

- adapts facedetection mojom Detect() to return an array<structs> 
 ISO a struct{ array<> }, causing a few changes throughout, including
 on Android impl (which doesn't support landmark detection);

- wires detection of said landmarks to Mac Face Detection impl and
 updates the associated unittests.



[1] https://github.com/WICG/shape-detection-api/issues/7
[2]
https://github.com/WICG/shape-detection-api/commit/dd90bf57313ea3d76b04c1b1c3...

BUG=718693

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[mcasas, 2017-05-05 15:18:26]

Patchset #1 (id:60001) has been deleted

[mcasas, 2017-05-05 15:18:53]

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

[mcasas, 2017-05-05 15:18:54]

scheib@ PTAL

[mcasas, 2017-05-05 17:42:16]

Patchset #1 (id:80001) has been deleted

[scheib, 2017-05-05 18:24:50]

lgtm

[mcasas, 2017-05-05 18:36:03]

mcasas@chromium.org changed reviewers:
+ avi@chromium.org, esprehn@chromium.org, tsepez@chromium.org

[mcasas, 2017-05-05 18:36:03]

esprehn@ RS plz additions to modules_idl_files.gni

tsepez@: RS plz catch-up to Spec in facedetection.mojom

avi@ RS plz catch-up to Spec in content/.../FaceDetectionImpl.java

[Avi (use Gerrit), 2017-05-05 19:06:54]

lgtm

stamp

[esprehn, 2017-05-05 19:23:10]

esprehn@chromium.org changed reviewers:
+ dglazkov@chromium.org

[esprehn, 2017-05-05 19:23:10]

lgtm, dglazkov@ FYI

[Tom Sepez, 2017-05-05 20:27:35]

mojom LGTM

[mcasas, 2017-05-05 20:29:37]

The CQ bit was checked by mcasas@chromium.org

[mcasas, 2017-05-05 20:29:37]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/2859413002/#ps110001
(title: "service-worker's global-interface-listing-expected.txt updated")

[commit-bot: I haz the power, 2017-05-05 20:30:49]

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, 2017-05-06 00:06:05]

CQ is committing da patch.

Bot data: {"patchset_id": 110001, "attempt_start_ts": 1494016177345630,
"parent_rev": "9974bff171e1248be9d6602d02465547a54ec682", "commit_rev":
"04f35409eaca0779dbe9b8435f20901cf510440b"}

[commit-bot: I haz the power, 2017-05-06 00:06:58]

Description was changed from

==========
Shape Detection: add idl and mojom for face landmarks and wire for Mac

Spec issue [1] and associated commit [2] added landmarks to the
detected faces, in particular eyes and mouth.  This CL:

-  adds the appropriate idls and mojo definitions to support those;

- adapts facedetection mojom Detect() to return an array<structs> 
 ISO a struct{ array<> }, causing a few changes throughout, including
 on Android impl (which doesn't support landmark detection);

- wires detection of said landmarks to Mac Face Detection impl and
 updates the associated unittests.



[1] https://github.com/WICG/shape-detection-api/issues/7
[2]
https://github.com/WICG/shape-detection-api/commit/dd90bf57313ea3d76b04c1b1c3...

BUG=718693

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Shape Detection: add idl and mojom for face landmarks and wire for Mac

Spec issue [1] and associated commit [2] added landmarks to the
detected faces, in particular eyes and mouth.  This CL:

-  adds the appropriate idls and mojo definitions to support those;

- adapts facedetection mojom Detect() to return an array<structs> 
 ISO a struct{ array<> }, causing a few changes throughout, including
 on Android impl (which doesn't support landmark detection);

- wires detection of said landmarks to Mac Face Detection impl and
 updates the associated unittests.



[1] https://github.com/WICG/shape-detection-api/issues/7
[2]
https://github.com/WICG/shape-detection-api/commit/dd90bf57313ea3d76b04c1b1c3...

BUG=718693

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.mac:mac_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2859413002
Cr-Commit-Position: refs/heads/master@{#469820}
Committed:
https://chromium.googlesource.com/chromium/src/+/04f35409eaca0779dbe9b8435f20...
==========

[commit-bot: I haz the power, 2017-05-06 00:07:00]

Committed patchset #2 (id:110001) as
https://chromium.googlesource.com/chromium/src/+/04f35409eaca0779dbe9b8435f20...

[haraken, 2017-05-06 14:43:07]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org, peter@chromium.org, yukishiino@chromium.org

[haraken, 2017-05-06 14:43:08]

modules/ LGTM, but would you add a test case to check that
DetectedFace::landmarks is really returning the same object?

I'm a bit afraid that it is not returning the same object because of a bug of
the current implementation of [SameObject] in the IDL compiler... The current
bindings creates a new v8::Value that wraps the returned HeapVector<Landmark>,
which ends up with returning a different v8 value every time you call
DetectedFace::landmarks.

+yukishiino
+peter FYI

[Yuki, 2017-05-08 09:34:12]

haraken@ is correct.

https://codereview.chromium.org/2859413002/diff/110001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/shapedetection/DetectedFace.idl (right):

https://codereview.chromium.org/2859413002/diff/110001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/shapedetection/DetectedFace.idl:14:
[SameObject] readonly attribute FrozenArray<Landmark> landmarks;
Please add [SaveSameObject].
  [SameObject, SaveSameObject] readonly attribute ...;

[mcasas, 2017-05-09 17:27:47]

On 2017/05/08 09:34:12, Yuki(ooo_til_may8) wrote:
> haraken@ is correct.
> 
>
https://codereview.chromium.org/2859413002/diff/110001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/shapedetection/DetectedFace.idl
(right):
> 
>
https://codereview.chromium.org/2859413002/diff/110001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/shapedetection/DetectedFace.idl:14:
> [SameObject] readonly attribute FrozenArray<Landmark> landmarks;
> Please add [SaveSameObject].
>   [SameObject, SaveSameObject] readonly attribute ...;

Thanks folks! Follow up in 
https://codereview.chromium.org/2873453004