Whitelist virtual keyboard extension to accept audioCapture and videoCapture permissions

Whitelist virtual keyboard extension to accept audioCapture and videoCapture permissions

Normally, audioCapture and videoCapture should only valid
for platform app. This CL whitelist virtual keyboard extension
to accepts audioCapture and videoCapture. It should fix the 
voice input regression that described in 289697. Note normal
extension that tries to use the two permissions above should 
still trigger warning message at chrome://extensions


BUG=289697
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=223946

[bshe, 2013-09-12 17:56:17]

Hi Sergey.

Would you mind take a look at this CL? Since the audioCapture is not intended to
be used by extension, this CL just whitelist virtual keyboard extension to
always be able to receive media stream.

Thanks!

[Sergey Ulanov, 2013-09-13 19:33:25]

As I mentioned on the bug I think the better approach would be to allow
audioCapture/videoCapture permission for extensions. You can fix it yourself if
you'd like (it should be rather simple change), or I will fix it myself when I
have time.

[bshe, 2013-09-13 20:11:49]

On 2013/09/13 19:33:25, Sergey Ulanov wrote:
> As I mentioned on the bug I think the better approach would be to allow
> audioCapture/videoCapture permission for extensions. You can fix it yourself
if
> you'd like (it should be rather simple change), or I will fix it myself when I
> have time.

I did a quick try to allow audioCapture and videoCapture permission for
extensions.
Would you mind to take a look? Thanks!

[Sergey Ulanov, 2013-09-13 21:06:04]

On 2013/09/13 20:11:49, bshe wrote:
> On 2013/09/13 19:33:25, Sergey Ulanov wrote:
> > As I mentioned on the bug I think the better approach would be to allow
> > audioCapture/videoCapture permission for extensions. You can fix it yourself
> if
> > you'd like (it should be rather simple change), or I will fix it myself when
I
> > have time.
> 
> I did a quick try to allow audioCapture and videoCapture permission for
> extensions.
> Would you mind to take a look? Thanks!

LGTM. Thanks! Can you please verify that the "My Chrome Theme" app still works
with this change? It's a packaged app so it should show infobar.
I'm not an owner for the json file - You'll need somebody from extension team
approve it.

[bshe, 2013-09-16 14:50:31]

Thanks for review. I verified that "My Chrome Theme" still works with this
change.

+miket for chrome/common/extensions/api/_permission_features.json

Thanks!

[miket_OOO, 2013-09-16 16:50:16]

I'd like to run this by meacer@ first. I'm not seeing from the linked history in
the crbug discussion that we've made an explicit decision to allow extensions to
capture audio/video with just a manifest permission. If a specific trusted
extension needs it, that's one thing. But that's not the only thing this CL
does. meacer@, what do you think?

[meacer, 2013-09-16 18:19:28]

On 2013/09/16 16:50:16, miket wrote:
> I'd like to run this by meacer@ first. I'm not seeing from the linked history
in
> the crbug discussion that we've made an explicit decision to allow extensions
to
> capture audio/video with just a manifest permission. If a specific trusted
> extension needs it, that's one thing. But that's not the only thing this CL
> does. meacer@, what do you think?

It looks like this is a big change that needs to go through API review first, if
I understand correctly. It's also unclear to me what the implications of
allowing
audioCapture and videoCapture to extensions are. For example, can an extension
start
audiocapture in its background script with this change? Would starting with a
very limited,
whitelist only approach with an API such as audioCapturePrivate help here?

[Sergey Ulanov, 2013-09-16 18:51:07]

On 2013/09/16 18:19:28, Mustafa Emre Acer wrote:
> On 2013/09/16 16:50:16, miket wrote:
> > I'd like to run this by meacer@ first. I'm not seeing from the linked
history
> in
> > the crbug discussion that we've made an explicit decision to allow
extensions
> to
> > capture audio/video with just a manifest permission. If a specific trusted
> > extension needs it, that's one thing. But that's not the only thing this CL
> > does. meacer@, what do you think?
> 
> It looks like this is a big change that needs to go through API review first,
if
> I understand correctly. It's also unclear to me what the implications of
> allowing
> audioCapture and videoCapture to extensions are. For example, can an extension
> start
> audiocapture in its background script with this change? Would starting with a
> very limited,
> whitelist only approach with an API such as audioCapturePrivate help here?

I think it should be possible to keep the same name "audioCapture", but allow it
only for whitelisted set of extensions.

[bshe, 2013-09-16 18:59:33]

On 2013/09/16 18:51:07, Sergey Ulanov wrote:
> On 2013/09/16 18:19:28, Mustafa Emre Acer wrote:
> > On 2013/09/16 16:50:16, miket wrote:
> > > I'd like to run this by meacer@ first. I'm not seeing from the linked
> history
> > in
> > > the crbug discussion that we've made an explicit decision to allow
> extensions
> > to
> > > capture audio/video with just a manifest permission. If a specific trusted
> > > extension needs it, that's one thing. But that's not the only thing this
CL
> > > does. meacer@, what do you think?
> > 
> > It looks like this is a big change that needs to go through API review
first,
> if
> > I understand correctly. It's also unclear to me what the implications of
> > allowing
> > audioCapture and videoCapture to extensions are. For example, can an
extension
> > start
> > audiocapture in its background script with this change? Would starting with
a
> > very limited,
> > whitelist only approach with an API such as audioCapturePrivate help here?
> 
> I think it should be possible to keep the same name "audioCapture", but allow
it
> only for whitelisted set of extensions.

Go through the review process may take some time. As a temporary solution, it
looks like
whitelist the extension with "audioCapture" permission is the easiest for
virtual keyboard.

I created Issue 292856 to track the process of exposing audioCapture and
videoCapture to extension.

[bshe, 2013-09-16 21:58:58]

I have update this issue and only whitelist VK extension. Would you guys mind to
do another round of review?

Thanks!

[Mustafa Acer, 2013-09-16 22:02:51]

On 2013/09/16 21:58:58, bshe wrote:
> I have update this issue and only whitelist VK extension. Would you guys mind
to
> do another round of review?
> 
> Thanks!

LGTM, thanks Biao!

[meacer, 2013-09-16 22:04:23]

On 2013/09/16 22:02:51, Mustafa Acer wrote:
> On 2013/09/16 21:58:58, bshe wrote:
> > I have update this issue and only whitelist VK extension. Would you guys
mind
> to
> > do another round of review?
> > 
> > Thanks!
> 
> LGTM, thanks Biao!

Another lgtm from chromium account just in case :)

[miket_OOO, 2013-09-17 17:13:40]

Much more comfortable with this narrow fix. LGTM.

https://codereview.chromium.org/23454030/diff/25001/chrome/common/extensions/...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/23454030/diff/25001/chrome/common/extensions/...
chrome/common/extensions/api/_permission_features.json:712: "videoCapture": [{
This whitespace style doesn't match the rest of the file. Please fix.

[bshe, 2013-09-18 01:41:50]

Thanks for review!

https://codereview.chromium.org/23454030/diff/25001/chrome/common/extensions/...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/23454030/diff/25001/chrome/common/extensions/...
chrome/common/extensions/api/_permission_features.json:712: "videoCapture": [{
On 2013/09/17 17:13:40, miket OOO until Sep 23 wrote:
> This whitespace style doesn't match the rest of the file. Please fix.

Done.

[commit-bot: I haz the power, 2013-09-18 17:35:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/23454030/33001

[commit-bot: I haz the power, 2013-09-18 21:50:53]

Change committed as 223946