Bring Audio Share and Tab Share Default On

chrome/browser/media/desktop_capture_access_handler.cc

Index: chrome/browser/media/desktop_capture_access_handler.cc
diff --git a/chrome/browser/media/desktop_capture_access_handler.cc b/chrome/browser/media/desktop_capture_access_handler.cc
index a9aa1bfb6be375e3ffa5c9aee2add8a5a6b20d5d..c1c36fbc2a891f2db39b13f86a4d1199d526b479 100644
--- a/chrome/browser/media/desktop_capture_access_handler.cc
+++ b/chrome/browser/media/desktop_capture_access_handler.cc
@@ -367,9 +367,9 @@ void DesktopCaptureAccessHandler::HandleRequest(
       media_id.type == content::DesktopMediaID::TYPE_WEB_CONTENTS;
 
   const bool has_flag = base::CommandLine::ForCurrentProcess()->HasSwitch(

[Sergey Ulanov, 2016/03/29 18:48:19]

See my next comment first, but if you decide to keep this flag then please
rename this variable. E.g. audio_support_disabled, or something like this.

[qiangchen, 2016/03/29 20:10:44]

Done.

I changed to check_audio_permission.

-      extensions::switches::kEnableDesktopCaptureAudio);
-  const bool capture_audio =
-      (has_flag ? audio_permitted : true) && audio_requested && audio_supported;
+      extensions::switches::kDisableDesktopCaptureAudio);
+  const bool capture_audio = (!has_flag ? audio_permitted : true) &&

[Sergey Ulanov, 2016/03/29 18:48:19]

This looks weird. capture_audio may still be set to true when the --disable flag
is specified. I think the problem here is that the name of the flag doesn't
reflect the meaning. It doesn't really disable audio capture for desktop
streams. Audio capture support is always enabled for desktop streams. The flag
disabled the new mechanism used to request audio permissions. Maybe rename the
flag to make the meaning clear?

But I'm not sure we need that flag at all. Can it be removed?

[qiangchen, 2016/03/29 20:10:44]

I found it weird also. Before my change, we do not support audio for desktop
sharing, but in the code, we can see it handles audio case. But I am not sure
whether this code path may serve some use cases unknown to me.

Original logic is capture_audio = audio_requested && audio_supported, and after
my change it should be capture_audio = audio_requested && audio_supported &&
audio_permitted.

Using tentative expression here is to make the logic exactly the same as before
when the enable-flag is off (or now the disable-flag is on).

[Sergey Ulanov, 2016/03/30 19:34:22]

Audio was supported  for rather long time. See crbug.com/223639. Chromecast
relies on that functionality when casting screen. The difference is that
previously chooseDesktopMedia() API allowed it implicitly, vs now there is an
explicit check-box for it.
Yes, chromecast uses that functionality when casting screen.
I'm not arguing that the logic is not correct. It looks correct. It's the name
of the flag that needs to be changed. Currently the flag doesn't do what the
name says.
E.g. it could be renamed to --disable-audio-permission-in-desktop-capture-api.

+      audio_requested && audio_supported;
 
   ui = GetDevicesForDesktopCapture(&devices, media_id, capture_audio, true,
                                    GetApplicationTitle(web_contents, extension),