VR: keep track of all pages that are recording audio/video/screen

VR: keep track of all pages that are recording audio/video/screen

When enter VR, keep track of all pages that are captureing audio/video/screen.

BUG=722836
Review-Url: https://codereview.chromium.org/2902813003
Cr-Commit-Position: refs/heads/master@{#474712}
Committed: https://chromium.googlesource.com/chromium/src/+/3410b8cf2a5048561552018a9b3ca10c14698a5a

[asimjour1, 2017-05-23 20:24:22]

asimjour@chromium.org changed reviewers:
+ cjgrant@chromium.org

[asimjour1, 2017-05-23 20:24:22]

PTAL

[cjgrant, 2017-05-23 20:38:19]

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:340: if
(indicator->IsCapturingAudio(tab->web_contents()))
If this method is reliable enough to use with other tabs, why not use it to
maintain the current tab's capture state as well?  This seems really clean.

[asimjour1, 2017-05-23 20:49:19]

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:340: if
(indicator->IsCapturingAudio(tab->web_contents()))
On 2017/05/23 20:38:18, cjgrant wrote:
> If this method is reliable enough to use with other tabs, why not use it to
> maintain the current tab's capture state as well?  This seems really clean.

Current tab can change its current state. So we have to poll the flag every 0.01
seconds. We don't need to do the same for the background tabs, so it will be
more efficient if we don't iterate on all tabs every 0.01 seconds.

[cjgrant, 2017-05-24 13:56:25]

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:340: if
(indicator->IsCapturingAudio(tab->web_contents()))
On 2017/05/23 20:49:19, asimjour1 wrote:
> On 2017/05/23 20:38:18, cjgrant wrote:
> > If this method is reliable enough to use with other tabs, why not use it to
> > maintain the current tab's capture state as well?  This seems really clean.
> 
> Current tab can change its current state. So we have to poll the flag every
0.01
> seconds. We don't need to do the same for the background tabs, so it will be
> more efficient if we don't iterate on all tabs every 0.01 seconds.

A few more questions:
- Can a background tab ever change state?
- This code explicitly skips the current tab when counting right?  If that's the
case, won't it eliminate the current tab from being counted?  I'll message you
offline to understand this better.

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:593: number_of_tabs_capturing_audio_
+= is_capturing_audio ? 1 : -1;
style guide prefers "num_tabs_capturing_audio_" (and shorter is better :)  Then
its:
ui_->setAudiocapturingIndicator(num_tabs_... > 0);

[asimjour1, 2017-05-25 14:38:37]

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/vr_shell.cc (right):

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:340: if
(indicator->IsCapturingAudio(tab->web_contents()))
On 2017/05/24 13:56:25, cjgrant wrote:
> On 2017/05/23 20:49:19, asimjour1 wrote:
> > On 2017/05/23 20:38:18, cjgrant wrote:
> > > If this method is reliable enough to use with other tabs, why not use it
to
> > > maintain the current tab's capture state as well?  This seems really
clean.
> > 
> > Current tab can change its current state. So we have to poll the flag every
> 0.01
> > seconds. We don't need to do the same for the background tabs, so it will be
> > more efficient if we don't iterate on all tabs every 0.01 seconds.
> 
> A few more questions:
> - Can a background tab ever change state?
> - This code explicitly skips the current tab when counting right?  If that's
the
> case, won't it eliminate the current tab from being counted?  I'll message you
> offline to understand this better.

I moved the code from here, and check all tabs every time.

https://codereview.chromium.org/2902813003/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/vr_shell.cc:593: number_of_tabs_capturing_audio_
+= is_capturing_audio ? 1 : -1;
On 2017/05/24 13:56:25, cjgrant wrote:
> style guide prefers "num_tabs_capturing_audio_" (and shorter is better :) 
Then
> its:
> ui_->setAudiocapturingIndicator(num_tabs_... > 0);

Done.

[cjgrant, 2017-05-25 14:57:44]

lgtm

[asimjour1, 2017-05-25 15:02:01]

The CQ bit was checked by asimjour@chromium.org

[commit-bot: I haz the power, 2017-05-25 15:02:29]

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-25 17:14:50]

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

[commit-bot: I haz the power, 2017-05-25 17:14:51]

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_...)

[asimjour1, 2017-05-25 18:14:23]

The CQ bit was checked by asimjour@chromium.org

[commit-bot: I haz the power, 2017-05-25 18:15:11]

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-25 18:25:20]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495736063050500,
"parent_rev": "beba06e1e5754b44ea3d3591e1564f8c327fe0bd", "commit_rev":
"3410b8cf2a5048561552018a9b3ca10c14698a5a"}

[commit-bot: I haz the power, 2017-05-25 18:25:40]

Description was changed from

==========
VR: keep track of all pages that are recording audio/video/screen

When enter VR, keep track of all pages that are captureing audio/video/screen.

BUG=722836
==========

to

==========
VR: keep track of all pages that are recording audio/video/screen

When enter VR, keep track of all pages that are captureing audio/video/screen.

BUG=722836

Review-Url: https://codereview.chromium.org/2902813003
Cr-Commit-Position: refs/heads/master@{#474712}
Committed:
https://chromium.googlesource.com/chromium/src/+/3410b8cf2a5048561552018a9b3c...
==========

[commit-bot: I haz the power, 2017-05-25 18:25:41]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/3410b8cf2a5048561552018a9b3c...