New issue
Advanced search Search tips

Issue 867533 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac , Fuchsia
Pri: 2
Type: Feature
Launch-Privacy: NA
Launch-Security: Yes
Launch-UI: NA



Sign in to add a comment

Extension API Modification: add a new method 'getMediaStreamId()' into tabCapture API

Project Member Reported by braveyao@chromium.org, Jul 25

Issue description

Extension API Modification Proposal
Extend tabCapture API with a new method 'getMediaStreamId()' to make
web apps can easily access the content of the current tab and
share/record with mic or camera data.

API Namespace: chrome.tabCapture
API Owners: braveyao@chromium.org, niklase@chromium.org
The following documents may not be necessary depending on the scope of your proposal:
API Overview Doc: https://docs.google.com/document/d/13P-CCysxsRHHW8l8P94YPUZ6nhAbqbn7d40DX4vaiI4/edit?usp=sharing
Design Doc: https://docs.google.com/document/d/1YWFmFJaN-bbAxh01JP5qczERW9vda62vLInO32WvCM8/edit?ts=5ac7ebd8
Supplementary Resources: 


 
This proposal has passed reviews from:

extension-api-reviews@, by rdcronin@

chrome-enamel@, by meacer@

Labels: -Launch-Privacy-NotReviewed -Launch-UI-NotReviewed -Launch-Security-NotReviewed M-70 Launch-Privacy-NA Launch-Security-ReviewRequested Launch-UI-NA
The reviews were done by email thread. Try to get sign-off here.

Flipping Launch-Security bit to ReviewRequested.
How to get API review sigh-off, which is a label instead of bits?

No UI change, nor user data concern.
Cc: rdevlin....@chromium.org mea...@chromium.org
+ rdvlin.cornin@ and meacer@
Excuse me, no idea about how to drive this. So add the reviewers from the email thread here.
Labels: -Launch-Security-ReviewRequested Launch-Security-Yes
Copying my response from the email thread:

"The most important distinction between tabCapture and desktopCapture is the install time permissions: 
- tabCapture implies access to all urls, triggering "read and modify your data on all sites" warning
- desktopCapture triggers "can capture contents of your screen" warning

Since tabCapture already implies access to all urls and since we can't prevent extensions from cooperating with websites that they have access to, there doesn't seem to be an additional risk in letting the website trigger the capture. That's already possible with the current tabCapture API, even though getting the stream is much harder without this change.

I also don't think the proposal changes the impact of a potential XSS: If a site was cooperating with a webpage and passing the captured stream to it by some means, an XSS on that site would still be able to observe captured contents.

(All that said, I feel that desktopCapture is the better API because it does a better job informing the user. tabCapture on the other hand is a bit opaque since it relies on an install time permission, and  unfortunately, install time permission dialogs have a very high clickthrough rate)"

Given that nobody objected my post, I'm flipping the security bit.
Labels: -Launch-API-NotReviewed Launch-API-Yes
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/17500caa18d32380ccf8584f3a388afc6331e7db

commit 17500caa18d32380ccf8584f3a388afc6331e7db
Author: Weiyong Yao <braveyao@chromium.org>
Date: Sat Sep 08 02:59:04 2018

[tabCapture] add a new getMediaStreamId method

This cl is to add a new getMediaStreamId method to tabCapture API.
The object is to offer easy access to the content of current tab for
sharing/recording together with mic/cam by web apps, by returning an
encrypted device id from extension to web app.
See design doc: https://docs.google.com/document/d/1YWFmFJaN-bbAxh01JP5qczERW9vda62vLInO32WvCM8/edit?ts=5ac7ebd8

TEST = autotests and a modified tab_capture example.

Bug:  867533 
Change-Id: I03283b2c6ce5187224e45370f8ca9b27881d570a
Reviewed-on: https://chromium-review.googlesource.com/1167659
Commit-Queue: Weiyong Yao <braveyao@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589762}
[modify] https://crrev.com/17500caa18d32380ccf8584f3a388afc6331e7db/chrome/browser/extensions/api/tab_capture/tab_capture_api.cc
[modify] https://crrev.com/17500caa18d32380ccf8584f3a388afc6331e7db/chrome/browser/extensions/api/tab_capture/tab_capture_api.h
[modify] https://crrev.com/17500caa18d32380ccf8584f3a388afc6331e7db/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc
[modify] https://crrev.com/17500caa18d32380ccf8584f3a388afc6331e7db/chrome/browser/media/webrtc/tab_capture_access_handler.cc
[modify] https://crrev.com/17500caa18d32380ccf8584f3a388afc6331e7db/chrome/common/extensions/api/tab_capture.idl
[modify] https://crrev.com/17500caa18d32380ccf8584f3a388afc6331e7db/chrome/test/data/extensions/api_test/tab_capture/api_tests.js
[modify] https://crrev.com/17500caa18d32380ccf8584f3a388afc6331e7db/extensions/browser/extension_function_histogram_value.h
[modify] https://crrev.com/17500caa18d32380ccf8584f3a388afc6331e7db/tools/metrics/histograms/enums.xml

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/36938ff06c51a36d01365beab00b114f7a9670c4

commit 36938ff06c51a36d01365beab00b114f7a9670c4
Author: Reid Kleckner <rnk@chromium.org>
Date: Mon Sep 10 19:11:33 2018

Revert "[tabCapture] add a new getMediaStreamId method"

This reverts commit 17500caa18d32380ccf8584f3a388afc6331e7db.

Reason for revert: Tests do not pass in official builds (https://crbug.com/882397)

Original change's description:
> [tabCapture] add a new getMediaStreamId method
>
> This cl is to add a new getMediaStreamId method to tabCapture API.
> The object is to offer easy access to the content of current tab for
> sharing/recording together with mic/cam by web apps, by returning an
> encrypted device id from extension to web app.
> See design doc: https://docs.google.com/document/d/1YWFmFJaN-bbAxh01JP5qczERW9vda62vLInO32WvCM8/edit?ts=5ac7ebd8
>
> TEST = autotests and a modified tab_capture example.
>
> Bug:  867533 
> Change-Id: I03283b2c6ce5187224e45370f8ca9b27881d570a
> Reviewed-on: https://chromium-review.googlesource.com/1167659
> Commit-Queue: Weiyong Yao <braveyao@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Reviewed-by: Yuri Wiitala <miu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#589762}

TBR=miu@chromium.org,rdevlin.cronin@chromium.org,braveyao@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  867533 , 882397
Change-Id: Ie45fd4c08a07b1c1b45e36be144340a7e8d8c1df
Reviewed-on: https://chromium-review.googlesource.com/1216807
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590012}
[modify] https://crrev.com/36938ff06c51a36d01365beab00b114f7a9670c4/chrome/browser/extensions/api/tab_capture/tab_capture_api.cc
[modify] https://crrev.com/36938ff06c51a36d01365beab00b114f7a9670c4/chrome/browser/extensions/api/tab_capture/tab_capture_api.h
[modify] https://crrev.com/36938ff06c51a36d01365beab00b114f7a9670c4/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc
[modify] https://crrev.com/36938ff06c51a36d01365beab00b114f7a9670c4/chrome/browser/media/webrtc/tab_capture_access_handler.cc
[modify] https://crrev.com/36938ff06c51a36d01365beab00b114f7a9670c4/chrome/common/extensions/api/tab_capture.idl
[modify] https://crrev.com/36938ff06c51a36d01365beab00b114f7a9670c4/chrome/test/data/extensions/api_test/tab_capture/api_tests.js
[modify] https://crrev.com/36938ff06c51a36d01365beab00b114f7a9670c4/extensions/browser/extension_function_histogram_value.h
[modify] https://crrev.com/36938ff06c51a36d01365beab00b114f7a9670c4/tools/metrics/histograms/enums.xml

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc87db8a3cd19aec3d4796d4fcc3b92e09e4a3fd

commit bc87db8a3cd19aec3d4796d4fcc3b92e09e4a3fd
Author: Weiyong Yao <braveyao@chromium.org>
Date: Wed Sep 12 16:11:47 2018

Give desktop capture extension constants unique names

We are reusing some codes between desktopCapture and tabCapture APIs.
And if those constants are with same name, then it will cause collisions
in some jumbo build configurations where they are compiled in the same
translation unit.

This cl gives the constants unique names for desktopCapture APIs
before relanding the tabCapture changes.

Bug:  867533 
Change-Id: I0d8369b1786bee0027e1fdf317f1d1d7915e52f9
Reviewed-on: https://chromium-review.googlesource.com/1219969
Reviewed-by: Daniel Bratell <bratell@opera.com>
Commit-Queue: Weiyong Yao <braveyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590710}
[modify] https://crrev.com/bc87db8a3cd19aec3d4796d4fcc3b92e09e4a3fd/chrome/browser/extensions/api/desktop_capture/desktop_capture_api.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b79156a434f215444989be10c007a1a992280108

commit b79156a434f215444989be10c007a1a992280108
Author: Weiyong Yao <braveyao@chromium.org>
Date: Wed Sep 12 17:50:29 2018

Reland "[tabCapture] add a new getMediaStreamId method"

This reverts commit 36938ff06c51a36d01365beab00b114f7a9670c4.

Reason for revert: The affected test case has been removed due to
being invalid now.

Original change's description:
> Revert "[tabCapture] add a new getMediaStreamId method"
> 
> This reverts commit 17500caa18d32380ccf8584f3a388afc6331e7db.
> 
> Reason for revert: Tests do not pass in official builds (https://crbug.com/882397)
> 
> Original change's description:
> > [tabCapture] add a new getMediaStreamId method
> >
> > This cl is to add a new getMediaStreamId method to tabCapture API.
> > The object is to offer easy access to the content of current tab for
> > sharing/recording together with mic/cam by web apps, by returning an
> > encrypted device id from extension to web app.
> > See design doc: https://docs.google.com/document/d/1YWFmFJaN-bbAxh01JP5qczERW9vda62vLInO32WvCM8/edit?ts=5ac7ebd8
> >
> > TEST = autotests and a modified tab_capture example.
> >
> > Bug:  867533 
> > Change-Id: I03283b2c6ce5187224e45370f8ca9b27881d570a
> > Reviewed-on: https://chromium-review.googlesource.com/1167659
> > Commit-Queue: Weiyong Yao <braveyao@chromium.org>
> > Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> > Reviewed-by: Yuri Wiitala <miu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#589762}
> 
> TBR=miu@chromium.org,rdevlin.cronin@chromium.org,braveyao@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  867533 , 882397
> Change-Id: Ie45fd4c08a07b1c1b45e36be144340a7e8d8c1df
> Reviewed-on: https://chromium-review.googlesource.com/1216807
> Commit-Queue: Reid Kleckner <rnk@chromium.org>
> Reviewed-by: Reid Kleckner <rnk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#590012}

TBR=miu@chromium.org,rnk@chromium.org,rdevlin.cronin@chromium.org,braveyao@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  867533 , 882397
Change-Id: I80ecbdbae04fdd1ace083c19f59b1398242f8e2b
Reviewed-on: https://chromium-review.googlesource.com/1221701
Reviewed-by: Reid Kleckner <rnk@chromium.org>
Reviewed-by: Weiyong Yao <braveyao@chromium.org>
Commit-Queue: Weiyong Yao <braveyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590747}
[modify] https://crrev.com/b79156a434f215444989be10c007a1a992280108/chrome/browser/extensions/api/tab_capture/tab_capture_api.cc
[modify] https://crrev.com/b79156a434f215444989be10c007a1a992280108/chrome/browser/extensions/api/tab_capture/tab_capture_api.h
[modify] https://crrev.com/b79156a434f215444989be10c007a1a992280108/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc
[modify] https://crrev.com/b79156a434f215444989be10c007a1a992280108/chrome/browser/media/webrtc/tab_capture_access_handler.cc
[modify] https://crrev.com/b79156a434f215444989be10c007a1a992280108/chrome/common/extensions/api/tab_capture.idl
[modify] https://crrev.com/b79156a434f215444989be10c007a1a992280108/chrome/test/data/extensions/api_test/tab_capture/api_tests.js
[modify] https://crrev.com/b79156a434f215444989be10c007a1a992280108/extensions/browser/extension_function_histogram_value.h
[modify] https://crrev.com/b79156a434f215444989be10c007a1a992280108/tools/metrics/histograms/enums.xml

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/afc92504b95ab84f9c1dae64b2935e82c3d5316b

commit afc92504b95ab84f9c1dae64b2935e82c3d5316b
Author: Weiyong Yao <braveyao@chromium.org>
Date: Thu Sep 13 17:04:40 2018

[tabCapture Sample]: add an option to choose the method to use for demo.

Recently we added a new method getMediaStreamId() into tabCapture api,
which works mostly like the existing capture() method but returns a
stream id instead of a stream. So it's good to have a sample for it to
demonstrate how to call getUserMedia() with the stream id.

This cl is to add an option to the tabCapture example to choose the
method to use for the tabCapture demo, which will record the last
choice and use it for the next demo. Otherwise the
tabCapture.capture() is the default.

Bug:  867533 
Change-Id: I8cb28aec0d8ccef98cb4a91993904045faa56f95
Reviewed-on: https://chromium-review.googlesource.com/1222879
Commit-Queue: Weiyong Yao <braveyao@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591050}
[modify] https://crrev.com/afc92504b95ab84f9c1dae64b2935e82c3d5316b/chrome/common/extensions/docs/examples/api/tabCapture/eventPage.js
[modify] https://crrev.com/afc92504b95ab84f9c1dae64b2935e82c3d5316b/chrome/common/extensions/docs/examples/api/tabCapture/manifest.json
[add] https://crrev.com/afc92504b95ab84f9c1dae64b2935e82c3d5316b/chrome/common/extensions/docs/examples/api/tabCapture/options.html
[add] https://crrev.com/afc92504b95ab84f9c1dae64b2935e82c3d5316b/chrome/common/extensions/docs/examples/api/tabCapture/options.js

Labels: -M-70 M-71
Status: Fixed (was: Assigned)

Sign in to add a comment