Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(5)

Issue 2860203002: MediaPerceptionPrivate IDL and skeleton. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 2 weeks ago by Luke Sorenson
Modified:
1 month ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, asvitkine+watch_chromium.org, extensions-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

MediaPerceptionPrivate IDL and skeleton. This private API is for process management of and communicating with a media analytics process running outside of Chrome. The analytics process uses computer vision technology to send metadata about image frames over D-Bus, and MediaPerception events are fired on the frontend. BUG=709180, 720495 Review-Url: https://codereview.chromium.org/2860203002 Cr-Commit-Position: refs/heads/master@{#473337} Committed: https://chromium.googlesource.com/chromium/src/+/aab09b896d22f03ccc701cfb8c6186df508405d5

Patch Set 1 #

Patch Set 2 : Addressing reviewer comments on dependant CL #

Patch Set 3 : Addressing comments on IDL. #

Total comments: 28

Patch Set 4 : Addressing more comments. #

Total comments: 8

Patch Set 5 : crbug for API whitelist. #

Total comments: 4

Patch Set 6 : Updating media perception private idl for GetDiagnostics. #

Patch Set 7 : Rename one field in IDL #

Total comments: 13

Patch Set 8 : Addressed reviewer comments. #

Patch Set 9 : Remove changes to webcam_private.idl #

Total comments: 6

Patch Set 10 : Changes to IDL and commit description. #

Total comments: 8

Patch Set 11 : Addressed comments and rebased with origin/master #

Patch Set 12 : Updating commit message. #

Total comments: 4

Patch Set 13 : Addressing final comments/ #

Patch Set 14 : Removing error statuses from idl. #

Patch Set 15 : Added depthInMeters to Entity dictionary. #

Total comments: 29

Patch Set 16 : Addressing Devlin's comments. #

Patch Set 17 : Adding Status UNKNOWN to idl. #

Patch Set 18 : I actually don't need UNKNOWN because STATUS_NONE exists by default. #

Patch Set 19 : Addressed Devlin's comments. #

Total comments: 4

Patch Set 20 : Addressed final comments and rebased with master. #

Patch Set 21 : Added permission set unittest fix. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+316 lines, -2 lines) Patch
M chrome/common/extensions/permissions/permission_set_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +1 line, -0 lines 0 comments Download
M extensions/browser/api/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +5 lines, -0 lines 0 comments Download
A extensions/browser/api/media_perception_private/OWNERS View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
A extensions/browser/api/media_perception_private/media_perception_private_api.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +62 lines, -0 lines 0 comments Download
A extensions/browser/api/media_perception_private/media_perception_private_api.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +42 lines, -0 lines 0 comments Download
M extensions/browser/extension_event_histogram_value.h View 1 chunk +1 line, -0 lines 0 comments Download
M extensions/browser/extension_function_histogram_value.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +3 lines, -0 lines 0 comments Download
M extensions/common/api/_api_features.json View 1 chunk +4 lines, -0 lines 0 comments Download
M extensions/common/api/_permission_features.json View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +18 lines, -2 lines 0 comments Download
A extensions/common/api/media_perception_private.idl View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +170 lines, -0 lines 0 comments Download
M extensions/common/api/schema.gni View 1 chunk +1 line, -0 lines 0 comments Download
M extensions/common/permissions/api_permission.h View 1 chunk +1 line, -0 lines 0 comments Download
M extensions/common/permissions/extensions_api_permissions.cc View 1 chunk +1 line, -0 lines 0 comments Download
M tools/metrics/histograms/enums.xml View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 3 chunks +5 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 46 (14 generated)
Luke Sorenson
CL 2/3 with the IDL definition, permissions and API skeleton.
1 month, 2 weeks ago (2017-05-08 19:27:08 UTC) #4
tbarzic
https://codereview.chromium.org/2860203002/diff/40001/extensions/common/api/_permission_features.json File extensions/common/api/_permission_features.json (right): https://codereview.chromium.org/2860203002/diff/40001/extensions/common/api/_permission_features.json#newcode255 extensions/common/api/_permission_features.json:255: // TODO(lasoren): Uncomment whitelist and make browser_tests work with ...
1 month, 2 weeks ago (2017-05-08 22:21:08 UTC) #5
Luke Sorenson
Thanks for the review tbarzic@ PTAL https://codereview.chromium.org/2860203002/diff/40001/extensions/common/api/_permission_features.json File extensions/common/api/_permission_features.json (right): https://codereview.chromium.org/2860203002/diff/40001/extensions/common/api/_permission_features.json#newcode255 extensions/common/api/_permission_features.json:255: // TODO(lasoren): Uncomment ...
1 month, 2 weeks ago (2017-05-09 17:41:05 UTC) #6
tbarzic
you should also add an owners file for extensions/broeser/api/media_perception_private/ https://codereview.chromium.org/2860203002/diff/40001/extensions/common/api/_permission_features.json File extensions/common/api/_permission_features.json (right): https://codereview.chromium.org/2860203002/diff/40001/extensions/common/api/_permission_features.json#newcode255 extensions/common/api/_permission_features.json:255: ...
1 month, 2 weeks ago (2017-05-09 21:03:12 UTC) #7
Luke Sorenson
Thanks, addressed comments! PTAL (uploading new patch set now) https://codereview.chromium.org/2860203002/diff/40001/extensions/common/api/_permission_features.json File extensions/common/api/_permission_features.json (right): https://codereview.chromium.org/2860203002/diff/40001/extensions/common/api/_permission_features.json#newcode255 extensions/common/api/_permission_features.json:255: ...
1 month, 1 week ago (2017-05-10 17:33:27 UTC) #8
tbarzic
https://codereview.chromium.org/2860203002/diff/60001/extensions/common/api/_permission_features.json File extensions/common/api/_permission_features.json (right): https://codereview.chromium.org/2860203002/diff/60001/extensions/common/api/_permission_features.json#newcode254 extensions/common/api/_permission_features.json:254: "extension_types": ["extension", "legacy_packaged_app", "platform_app"], On 2017/05/10 17:33:26, Luke Sorenson ...
1 month, 1 week ago (2017-05-10 23:45:53 UTC) #9
Luke Sorenson
Addressed comments, PTAL (uploading new patch set now) https://codereview.chromium.org/2860203002/diff/60001/extensions/common/api/_permission_features.json File extensions/common/api/_permission_features.json (right): https://codereview.chromium.org/2860203002/diff/60001/extensions/common/api/_permission_features.json#newcode254 extensions/common/api/_permission_features.json:254: "extension_types": ...
1 month, 1 week ago (2017-05-11 16:18:22 UTC) #10
tbarzic
few more nits, but looks OK https://codereview.chromium.org/2860203002/diff/120001/extensions/common/api/media_perception_private.idl File extensions/common/api/media_perception_private.idl (right): https://codereview.chromium.org/2860203002/diff/120001/extensions/common/api/media_perception_private.idl#newcode42 extensions/common/api/media_perception_private.idl:42: // Can be ...
1 month, 1 week ago (2017-05-11 17:16:21 UTC) #11
tbarzic
https://codereview.chromium.org/2860203002/diff/160001/extensions/common/api/_permission_features.json File extensions/common/api/_permission_features.json (right): https://codereview.chromium.org/2860203002/diff/160001/extensions/common/api/_permission_features.json#newcode257 extensions/common/api/_permission_features.json:257: "E703483CEF33DEC18B4B6DD84B5C776FB9182BDB", // http://crbug.com/720495 can you also link this bug ...
1 month, 1 week ago (2017-05-11 17:53:03 UTC) #12
Luke Sorenson
Addressed comments. PTAL https://codereview.chromium.org/2860203002/diff/120001/extensions/common/api/media_perception_private.idl File extensions/common/api/media_perception_private.idl (right): https://codereview.chromium.org/2860203002/diff/120001/extensions/common/api/media_perception_private.idl#newcode23 extensions/common/api/media_perception_private.idl:23: // Note: STARTED is the initial ...
1 month, 1 week ago (2017-05-11 18:35:06 UTC) #14
tbarzic
https://codereview.chromium.org/2860203002/diff/180001/extensions/browser/api/media_perception_private/BUILD.gn File extensions/browser/api/media_perception_private/BUILD.gn (right): https://codereview.chromium.org/2860203002/diff/180001/extensions/browser/api/media_perception_private/BUILD.gn#newcode15 extensions/browser/api/media_perception_private/BUILD.gn:15: "//chromeos:media_perception_proto", you don't need this here - can you ...
1 month, 1 week ago (2017-05-11 19:20:03 UTC) #15
tbarzic
forgot to mention, bugs should be listed as: BUG=709180, 720495
1 month, 1 week ago (2017-05-11 19:38:50 UTC) #16
Luke Sorenson
Addressed comments and rebased with origin/master. PTAL https://codereview.chromium.org/2860203002/diff/180001/extensions/browser/api/media_perception_private/BUILD.gn File extensions/browser/api/media_perception_private/BUILD.gn (right): https://codereview.chromium.org/2860203002/diff/180001/extensions/browser/api/media_perception_private/BUILD.gn#newcode15 extensions/browser/api/media_perception_private/BUILD.gn:15: "//chromeos:media_perception_proto", On ...
1 month, 1 week ago (2017-05-11 23:28:17 UTC) #17
tbarzic
also, you should run: python tools/metrics/histograms/update_extension_permission.py other than that, lgtm https://codereview.chromium.org/2860203002/diff/180001/extensions/common/api/media_perception_private.idl File extensions/common/api/media_perception_private.idl (right): https://codereview.chromium.org/2860203002/diff/180001/extensions/common/api/media_perception_private.idl#newcode24 ...
1 month, 1 week ago (2017-05-11 23:42:50 UTC) #19
Luke Sorenson
Done. Thanks for the review! https://codereview.chromium.org/2860203002/diff/180001/extensions/common/api/media_perception_private.idl File extensions/common/api/media_perception_private.idl (right): https://codereview.chromium.org/2860203002/diff/180001/extensions/common/api/media_perception_private.idl#newcode24 extensions/common/api/media_perception_private.idl:24: // When the process ...
1 month, 1 week ago (2017-05-11 23:50:27 UTC) #20
Luke Sorenson
Adding Devlin for owner lgtm.
1 month, 1 week ago (2017-05-15 20:04:24 UTC) #22
Devlin
On 2017/05/15 20:04:24, Luke Sorenson wrote: > Adding Devlin for owner lgtm. Does this have ...
1 month ago (2017-05-17 14:46:22 UTC) #23
Luke Sorenson
On 2017/05/17 14:46:22, Devlin (catching up) wrote: > On 2017/05/15 20:04:24, Luke Sorenson wrote: > ...
1 month ago (2017-05-17 15:00:19 UTC) #24
Devlin
https://codereview.chromium.org/2860203002/diff/280001/extensions/browser/api/media_perception_private/BUILD.gn File extensions/browser/api/media_perception_private/BUILD.gn (right): https://codereview.chromium.org/2860203002/diff/280001/extensions/browser/api/media_perception_private/BUILD.gn#newcode1 extensions/browser/api/media_perception_private/BUILD.gn:1: # Copyright 2017 The Chromium Authors. All rights reserved. ...
1 month ago (2017-05-17 20:22:42 UTC) #25
Luke Sorenson
Thanks Devlin! Addressed comments. PTAL https://codereview.chromium.org/2860203002/diff/280001/extensions/browser/api/media_perception_private/BUILD.gn File extensions/browser/api/media_perception_private/BUILD.gn (right): https://codereview.chromium.org/2860203002/diff/280001/extensions/browser/api/media_perception_private/BUILD.gn#newcode1 extensions/browser/api/media_perception_private/BUILD.gn:1: # Copyright 2017 The ...
1 month ago (2017-05-17 21:01:36 UTC) #26
Luke Sorenson
Adding isherman@ for histograms files OWNER review.
1 month ago (2017-05-18 21:03:43 UTC) #28
Devlin
https://codereview.chromium.org/2860203002/diff/280001/extensions/browser/api/media_perception_private/BUILD.gn File extensions/browser/api/media_perception_private/BUILD.gn (right): https://codereview.chromium.org/2860203002/diff/280001/extensions/browser/api/media_perception_private/BUILD.gn#newcode1 extensions/browser/api/media_perception_private/BUILD.gn:1: # Copyright 2017 The Chromium Authors. All rights reserved. ...
1 month ago (2017-05-18 21:21:24 UTC) #29
Ilya Sherman
enum changes LGTM, thanks
1 month ago (2017-05-18 21:46:56 UTC) #30
Luke Sorenson
Thanks Devlin, addressed comments. PTAL Thanks Ilya for the LGTM! https://codereview.chromium.org/2860203002/diff/280001/extensions/browser/api/media_perception_private/BUILD.gn File extensions/browser/api/media_perception_private/BUILD.gn (right): https://codereview.chromium.org/2860203002/diff/280001/extensions/browser/api/media_perception_private/BUILD.gn#newcode1 ...
1 month ago (2017-05-18 21:56:33 UTC) #31
Devlin
https://codereview.chromium.org/2860203002/diff/280001/extensions/common/api/media_perception_private.idl File extensions/common/api/media_perception_private.idl (right): https://codereview.chromium.org/2860203002/diff/280001/extensions/common/api/media_perception_private.idl#newcode153 extensions/common/api/media_perception_private.idl:153: // |callback| : The State of the system after ...
1 month ago (2017-05-19 15:08:13 UTC) #32
Luke Sorenson
Thanks Devlin, replied to your comment https://codereview.chromium.org/2860203002/diff/280001/extensions/common/api/media_perception_private.idl File extensions/common/api/media_perception_private.idl (right): https://codereview.chromium.org/2860203002/diff/280001/extensions/common/api/media_perception_private.idl#newcode153 extensions/common/api/media_perception_private.idl:153: // |callback| : ...
1 month ago (2017-05-19 15:51:05 UTC) #33
Devlin
lgtm https://codereview.chromium.org/2860203002/diff/280001/extensions/common/api/media_perception_private.idl File extensions/common/api/media_perception_private.idl (right): https://codereview.chromium.org/2860203002/diff/280001/extensions/common/api/media_perception_private.idl#newcode153 extensions/common/api/media_perception_private.idl:153: // |callback| : The State of the system ...
1 month ago (2017-05-19 15:58:17 UTC) #34
Luke Sorenson
Thanks for the lgtm Devlin! Addressed final comments. https://codereview.chromium.org/2860203002/diff/360001/extensions/browser/api/media_perception_private/media_perception_private_api.cc File extensions/browser/api/media_perception_private/media_perception_private_api.cc (right): https://codereview.chromium.org/2860203002/diff/360001/extensions/browser/api/media_perception_private/media_perception_private_api.cc#newcode7 extensions/browser/api/media_perception_private/media_perception_private_api.cc:7: #include ...
1 month ago (2017-05-19 16:30:39 UTC) #35
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2860203002/380001
1 month ago (2017-05-19 16:33:19 UTC) #38
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/388820)
1 month ago (2017-05-19 17:20:32 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2860203002/400001
1 month ago (2017-05-19 20:08:27 UTC) #43
commit-bot: I haz the power
1 month ago (2017-05-19 21:42:22 UTC) #46
Message was sent while issue was closed.
Committed patchset #21 (id:400001) as
https://chromium.googlesource.com/chromium/src/+/aab09b896d22f03ccc701cfb8c61...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cb946e318