ShapeDetection: Add ShapeDetectionProvider

ShapeDetection: Add ShapeDetectionProvider as a factory for FaceDetection
service.

Remove |FaceDetectorOptions| from FaceDetection mojo interface so Blink
doesn't need to send it every time detect() is called.

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

Committed: https://crrev.com/b68a77d89a69697c9e376fbaca9a9db07b4896eb
Cr-Commit-Position: refs/heads/master@{#438582}

[xianglu, 2016-12-09 23:36:18]

Description was changed from

==========
Add ShapeDetectionProvider mojom service.

BUG=646083
==========

to

==========
Add ShapeDetectionProvider mojom service.

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

[xianglu, 2016-12-09 23:41:47]

Patchset #1 (id:1) has been deleted

[xianglu, 2016-12-10 00:07:41]

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

[commit-bot: I haz the power, 2016-12-10 00:08:57]

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

[xianglu, 2016-12-10 00:12:24]

Description was changed from

==========
Add ShapeDetectionProvider mojom service.

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

to

==========
ShapeDetection: Add ShapeDetectionProvider.

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

[xianglu, 2016-12-10 00:48:03]

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

[xianglu, 2016-12-10 00:48:08]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-12-10 00:48:47]

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

[xianglu, 2016-12-10 01:02:22]

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

[xianglu, 2016-12-10 01:02:47]

Patchset #1 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-12-10 01:03:59]

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

[xianglu, 2016-12-10 01:06:27]

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

[xianglu, 2016-12-10 01:06:28]

PTAL.

[xianglu, 2016-12-10 01:09:04]

Description was changed from

==========
ShapeDetection: Add ShapeDetectionProvider.

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

to

==========
ShapeDetection: Add ShapeDetectionProvider as a factory for FaceDetection
service. Remove |FaceDetectorOptions| from FaceDetection mojo interface so Blink
doesn't need to send it every time user calls detect().

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

[xianglu, 2016-12-10 01:12:35]

Description was changed from

==========
ShapeDetection: Add ShapeDetectionProvider as a factory for FaceDetection
service. Remove |FaceDetectorOptions| from FaceDetection mojo interface so Blink
doesn't need to send it every time user calls detect().

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

to

==========
ShapeDetection: Add ShapeDetectionProvider as a factory for FaceDetection
service. 

Remove |FaceDetectorOptions| from FaceDetection mojo interface so Blink
doesn't need to send it every time detect() is called.

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

[Reilly Grant (use Gerrit), 2016-12-10 01:18:22]

https://codereview.chromium.org/2564163003/diff/60001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java
(right):

https://codereview.chromium.org/2564163003/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:35:
mMaxFaces = options.maxDetectedFaces;
Since this value is attacker controlled should we should probably still enforce
some maximum.

https://codereview.chromium.org/2564163003/diff/60001/content/public/app/mojo...
File content/public/app/mojo/content_browser_manifest.json (left):

https://codereview.chromium.org/2564163003/diff/60001/content/public/app/mojo...
content/public/app/mojo/content_browser_manifest.json:16:
"blink::mojom::BarcodeDetection",
I don't see a CreateBarcodeDetector method in ShapeDetectionProvider so isn't
this service still requested directly?

[xianglu, 2016-12-10 02:04:46]

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

[commit-bot: I haz the power, 2016-12-10 02:05:25]

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

[xianglu, 2016-12-10 02:20:41]

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

[commit-bot: I haz the power, 2016-12-10 02:21:21]

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

[xianglu, 2016-12-10 02:21:27]

Patchset #2 (id:80001) has been deleted

[xianglu, 2016-12-10 02:22:19]

PTAL.

https://codereview.chromium.org/2564163003/diff/60001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java
(right):

https://codereview.chromium.org/2564163003/diff/60001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:35:
mMaxFaces = options.maxDetectedFaces;
On 2016/12/10 01:18:22, Reilly Grant wrote:
> Since this value is attacker controlled should we should probably still
enforce
> some maximum.

Done.

https://codereview.chromium.org/2564163003/diff/60001/content/public/app/mojo...
File content/public/app/mojo/content_browser_manifest.json (left):

https://codereview.chromium.org/2564163003/diff/60001/content/public/app/mojo...
content/public/app/mojo/content_browser_manifest.json:16:
"blink::mojom::BarcodeDetection",
On 2016/12/10 01:18:22, Reilly Grant wrote:
> I don't see a CreateBarcodeDetector method in ShapeDetectionProvider so isn't
> this service still requested directly?

Yes. BarcodeDetection is implemented under //chrome and created directly.
Changed name to FaceDetectionProvider.

[commit-bot: I haz the power, 2016-12-10 04:03:03]

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

[commit-bot: I haz the power, 2016-12-10 04:03:04]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[xianglu, 2016-12-12 21:30:28]

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

[commit-bot: I haz the power, 2016-12-12 21:33:31]

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

[xianglu, 2016-12-12 21:33:33]

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

[xianglu, 2016-12-12 21:33:38]

Patchset #3 (id:120001) has been deleted

[commit-bot: I haz the power, 2016-12-12 21:35:06]

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

[xianglu, 2016-12-12 21:35:31]

Patchset #3 (id:140001) has been deleted

[xianglu, 2016-12-12 21:35:37]

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

[xianglu, 2016-12-12 21:36:59]

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

[xianglu, 2016-12-12 21:37:07]

Patchset #3 (id:160001) has been deleted

[commit-bot: I haz the power, 2016-12-12 21:38:22]

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

[Reilly Grant (use Gerrit), 2016-12-12 21:41:30]

lgtm

[xianglu, 2016-12-12 21:51:22]

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

[commit-bot: I haz the power, 2016-12-12 21:52:25]

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-12-13 01:05:17]

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

[commit-bot: I haz the power, 2016-12-13 01:05:18]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[xianglu, 2016-12-13 01:19:31]

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

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

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-12-13 04:10:20]

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

[commit-bot: I haz the power, 2016-12-13 04:10:21]

Dry run: This issue passed the CQ dry run.

[mcasas, 2016-12-13 17:37:33]

lgtm % comments below.

Also, feel free to remove the patchsets that
have no comments on.

https://codereview.chromium.org/2564163003/diff/200001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java
(right):

https://codereview.chromium.org/2564163003/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:37:
mMaxFaces = Math.min(options.maxDetectedFaces, 32);
s/32/MAX_FACES/, with |MAX_FACES| as it was before.

https://codereview.chromium.org/2564163003/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:90:
// findFaces() will stop at mMaxFaces.
Escape variable names in comments: |mMaxFaces|

https://codereview.chromium.org/2564163003/diff/200001/content/public/app/moj...
File content/public/app/mojo/content_browser_manifest.json (left):

https://codereview.chromium.org/2564163003/diff/200001/content/public/app/moj...
content/public/app/mojo/content_browser_manifest.json:16:
"blink::mojom::BarcodeDetection",
IIRC this was needed to allow LayoutTests to use
a mock barcode detection service. If you have
verified that barcode detection still works, though,
then never mind this comment :-)

https://codereview.chromium.org/2564163003/diff/200001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/shapedetection/resources/mock-facedetection.js
(right):

https://codereview.chromium.org/2564163003/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/shapedetection/resources/mock-facedetection.js:3:
let mockFaceDetectionReady = define(
s/mockFaceDetectionReady/mockFaceDetectionProviderReady/

https://codereview.chromium.org/2564163003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/shapedetection/FaceDetector.h (right):

https://codereview.chromium.org/2564163003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/shapedetection/FaceDetector.h:13: #include
"modules/shapedetection/FaceDetectorOptions.h"
Not needed here? Move to the .cpp file.

[xianglu, 2016-12-13 19:06:15]

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

[commit-bot: I haz the power, 2016-12-13 19:06:57]

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

[xianglu, 2016-12-13 19:07:45]

Patchset #3 (id:180001) has been deleted

[xianglu, 2016-12-13 19:08:03]

Patchset #4 (id:220001) has been deleted

[xianglu, 2016-12-13 19:11:37]

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

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

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

[xianglu, 2016-12-13 19:14:09]

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

[xianglu, 2016-12-13 19:14:19]

Patchset #5 (id:260001) has been deleted

[xianglu, 2016-12-13 19:14:32]

Patchset #4 (id:240001) has been deleted

[commit-bot: I haz the power, 2016-12-13 19:14:42]

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

[xianglu, 2016-12-13 19:17:27]

xianglu@chromium.org changed reviewers:
+ avi@chromium.org, haraken@chromium.org, tsepez@chromium.org

[xianglu, 2016-12-13 19:17:28]

PTAL.

https://codereview.chromium.org/2564163003/diff/200001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java
(right):

https://codereview.chromium.org/2564163003/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:37:
mMaxFaces = Math.min(options.maxDetectedFaces, 32);
On 2016/12/13 17:37:32, mcasas wrote:
> s/32/MAX_FACES/, with |MAX_FACES| as it was before.

Done.

https://codereview.chromium.org/2564163003/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/shapedetection/FaceDetectionImpl.java:90:
// findFaces() will stop at mMaxFaces.
On 2016/12/13 17:37:32, mcasas wrote:
> Escape variable names in comments: |mMaxFaces|

Done.

https://codereview.chromium.org/2564163003/diff/200001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/shapedetection/resources/mock-facedetection.js
(right):

https://codereview.chromium.org/2564163003/diff/200001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/shapedetection/resources/mock-facedetection.js:3:
let mockFaceDetectionReady = define(
On 2016/12/13 17:37:32, mcasas wrote:
> s/mockFaceDetectionReady/mockFaceDetectionProviderReady/

Done.

https://codereview.chromium.org/2564163003/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/shapedetection/FaceDetector.h (right):

https://codereview.chromium.org/2564163003/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/shapedetection/FaceDetector.h:13: #include
"modules/shapedetection/FaceDetectorOptions.h"
On 2016/12/13 17:37:32, mcasas wrote:
> Not needed here? Move to the .cpp file.

Done.

[Reilly Grant (use Gerrit), 2016-12-13 19:21:44]

still lgtm

[Tom Sepez, 2016-12-13 19:23:24]

mojom lgtm

[commit-bot: I haz the power, 2016-12-13 20:35:52]

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

[commit-bot: I haz the power, 2016-12-13 20:35:54]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[xianglu, 2016-12-13 20:44:47]

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

[commit-bot: I haz the power, 2016-12-13 20:45:29]

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

[xianglu, 2016-12-13 20:46:40]

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

[commit-bot: I haz the power, 2016-12-13 20:46:56]

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

[xianglu, 2016-12-13 20:46:59]

Patchset #5 (id:300001) has been deleted

[commit-bot: I haz the power, 2016-12-13 22:35:22]

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

[commit-bot: I haz the power, 2016-12-13 22:35:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[xianglu, 2016-12-14 02:10:03]

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

[commit-bot: I haz the power, 2016-12-14 02:10:37]

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

[xianglu, 2016-12-14 02:15:21]

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

[xianglu, 2016-12-14 02:15:59]

Patchset #6 (id:330001) has been deleted

[commit-bot: I haz the power, 2016-12-14 02:16:05]

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

[xianglu, 2016-12-14 02:16:07]

Patchset #4 (id:280001) has been deleted

[commit-bot: I haz the power, 2016-12-14 04:16:32]

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

[commit-bot: I haz the power, 2016-12-14 04:16:33]

Dry run: This issue passed the CQ dry run.

[xianglu, 2016-12-14 08:49:30]

Patchset #4 (id:310001) has been deleted

[haraken, 2016-12-14 08:56:38]

WebKit LGTM

[Avi (use Gerrit), 2016-12-14 16:56:19]

lgtm

stamp

[xianglu, 2016-12-14 17:12:44]

The CQ bit was checked by xianglu@chromium.org

[xianglu, 2016-12-14 17:12:45]

The patchset sent to the CQ was uploaded after l-g-t-m from mcasas@chromium.org,
tsepez@chromium.org, reillyg@chromium.org
Link to the patchset: https://codereview.chromium.org/2564163003/#ps350001
(title: "Name change: mockFaceDetectionProviderReady, rebase")

[xianglu, 2016-12-14 17:12:53]

The CQ bit was unchecked by xianglu@chromium.org

[xianglu, 2016-12-14 17:13:04]

The CQ bit was checked by xianglu@chromium.org

[commit-bot: I haz the power, 2016-12-14 17:13:40]

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-12-14 19:21:24]

CQ is committing da patch.

Bot data: {"patchset_id": 350001, "attempt_start_ts": 1481735584015600,
"parent_rev": "8ecc0b1fe187a7dc0389ea96eae3bf82391b013b", "commit_rev":
"d9c06a47c376f79be54de5e3076521356c7a26e7"}

[commit-bot: I haz the power, 2016-12-14 19:21:49]

Description was changed from

==========
ShapeDetection: Add ShapeDetectionProvider as a factory for FaceDetection
service. 

Remove |FaceDetectorOptions| from FaceDetection mojo interface so Blink
doesn't need to send it every time detect() is called.

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

to

==========
ShapeDetection: Add ShapeDetectionProvider as a factory for FaceDetection
service. 

Remove |FaceDetectorOptions| from FaceDetection mojo interface so Blink
doesn't need to send it every time detect() is called.

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

Review-Url: https://codereview.chromium.org/2564163003
==========

[commit-bot: I haz the power, 2016-12-14 19:21:52]

Committed patchset #4 (id:350001)

[commit-bot: I haz the power, 2016-12-14 19:25:44]

Description was changed from

==========
ShapeDetection: Add ShapeDetectionProvider as a factory for FaceDetection
service. 

Remove |FaceDetectorOptions| from FaceDetection mojo interface so Blink
doesn't need to send it every time detect() is called.

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

Review-Url: https://codereview.chromium.org/2564163003
==========

to

==========
ShapeDetection: Add ShapeDetectionProvider as a factory for FaceDetection
service.

Remove |FaceDetectorOptions| from FaceDetection mojo interface so Blink
doesn't need to send it every time detect() is called.

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

Committed: https://crrev.com/b68a77d89a69697c9e376fbaca9a9db07b4896eb
Cr-Commit-Position: refs/heads/master@{#438582}
==========

[commit-bot: I haz the power, 2016-12-14 19:25:46]

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