Stop using the MEDIASTREAM content setting.

Stop using the MEDIASTREAM content setting.

While the exceptions of the deprecated MEDIASTREAM are no longer used (and were migrated to MEDIASTREAM_MIC and MEDIASTREAM_CAMERA exceptions), the default value is still used.

This CL:
1. Replaces MEDIASTREAM default checks with MIC or CAMERA checks in contexts where only one of the two permissions is required.
2. Replaces MEDIASTREAM default checks with both MIC and CAMERA in contexts where both of the permissions are required.
3. Replaces any write of MEDIASTREAM default with write of both MIC and CAMERA.

This CL does not get rid of the MEDIASTREAM content setting entirely. This is because:
1. There is a mapping between content settings and sections in chrome://settings/content. The media part is mapped to MEDIASTREAM.
2. There is a mapping between content settings and icons shown in the omnibox when the content settings are queried. The icon shown for both camera and microphone is mapped to MEDIASTREAM.

However, after this CL is applied, the MEDIASTREAM content setting will only be used in the above circumstances, never to set or get a value.

To avoid complicated syncing between MEDIASTREAM value on old versions and MEDIASTREAM_MIC, MEDIASTREAM_CAMERA on new versions of Chrome, this CL will not land until crbug.com/452388 is resolved.

TESTED=UI,policy
BUG=464382
Committed: https://crrev.com/f90963538b234e2bad01b2f182ba8d98b179b1a5
Cr-Commit-Position: refs/heads/master@{#322364}

[msramek, 2015-03-05 16:19:37]

msramek@chromium.org changed reviewers:
+ markusheintz@chromium.org

[msramek, 2015-03-05 16:19:38]

Hi Markus,

could you have a preliminary look at this CL, before I send it to other owners?

Thanks,
Martin

https://codereview.chromium.org/965103004/diff/40001/chrome/browser/media/med...
File chrome/browser/media/media_stream_device_permissions.cc (right):

https://codereview.chromium.org/965103004/diff/40001/chrome/browser/media/med...
chrome/browser/media/media_stream_device_permissions.cc:41: DCHECK(type ==
CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC ||
The previous implementation which used MEDIASTREAM should be semantically
equivalent to a conjunction:

CheckAllowAllMediaStreamContentForOrigin() {
  return ShouldAllowAllContent(MIC) && ShouldAllowAllContent(CAMERA);
}

However, the callers of this function want either audio or microphone, not both,
so I added another parameter to request the particular setting they need.

Note that this is just a formal change - ShouldAllowAllContent() currently
returns the same values for MEDIASTREAM, MEDIASTREAM_MIC and MEDIASTREAM_CAMERA
anyway.

https://codereview.chromium.org/965103004/diff/40001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/content_settings_handler.cc (right):

https://codereview.chromium.org/965103004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/content_settings_handler.cc:116:
{CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC, "media-stream"},
Nothing in the UI is currently labeled "media-stream-mic" or
"media-stream-camera", so we're not breaking anything here.

The point is that when we update the radio buttons for either MIC or CAMERA, we
want to refer to the pair of radio buttons that previously belonged to
MEDIASTREAM.

https://codereview.chromium.org/965103004/diff/40001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/content_settings_handler.cc:1364: if
(content_type == CONTENT_SETTINGS_TYPE_MEDIASTREAM) {
Unfortunately, we create a lot of additional complexity in the UI code. However,
this is only temporary, as we already have plans to demonstrate the split of
MEDIASTREAM to MIC and CAMERA in the UI as well.

https://codereview.chromium.org/965103004/diff/40001/components/content_setti...
File
components/content_settings/core/browser/content_settings_policy_provider.cc
(right):

https://codereview.chromium.org/965103004/diff/40001/components/content_setti...
components/content_settings/core/browser/content_settings_policy_provider.cc:36:
prefs::kManagedDefaultMediaStreamSetting,
If a policy sets kManagedDefaultMediaStreamSetting, then we should update both
MIC and CAMERA, but not MEDIASTREAM.

[msramek, 2015-03-06 10:36:55]

I somehow forgot that even if we stop syncing the media settings, we still need
the migration code. Added in the 4th patch.

Some changes related to supervised users provider appeared during rebase as
well.

[markusheintz_, 2015-03-06 16:05:33]

https://codereview.chromium.org/965103004/diff/60001/chrome/browser/media/med...
File chrome/browser/media/media_stream_device_permissions.cc (right):

https://codereview.chromium.org/965103004/diff/60001/chrome/browser/media/med...
chrome/browser/media/media_stream_device_permissions.cc:38: bool
CheckAllowAllMediaStreamContentForOrigin(Profile* profile,
Do we actually need this method anymore? Seems like one step of indirection we
could get rid of.

https://codereview.chromium.org/965103004/diff/60001/chrome/browser/media/med...
File chrome/browser/media/media_stream_devices_controller.cc (right):

https://codereview.chromium.org/965103004/diff/60001/chrome/browser/media/med...
chrome/browser/media/media_stream_devices_controller.cc:595: ) ==
CONTENT_SETTING_BLOCK &&
The closing ) should be in the previous line. 

Here and below

https://codereview.chromium.org/965103004/diff/60001/components/content_setti...
File
components/content_settings/core/browser/content_settings_default_provider.cc
(right):

https://codereview.chromium.org/965103004/diff/60001/components/content_setti...
components/content_settings/core/browser/content_settings_default_provider.cc:354:
default_settings_[CONTENT_SETTINGS_TYPE_MEDIASTREAM]->GetAsInteger(
Do the default_settings_ contain values for settings that were never changed by
the user?

[msramek, 2015-03-06 17:28:49]

Patchset #5 (id:80001) has been deleted

[msramek, 2015-03-06 17:47:33]

https://codereview.chromium.org/965103004/diff/60001/chrome/browser/media/med...
File chrome/browser/media/media_stream_device_permissions.cc (right):

https://codereview.chromium.org/965103004/diff/60001/chrome/browser/media/med...
chrome/browser/media/media_stream_device_permissions.cc:38: bool
CheckAllowAllMediaStreamContentForOrigin(Profile* profile,
On 2015/03/06 16:05:33, markusheintz_ wrote:
> Do we actually need this method anymore? Seems like one step of indirection we
> could get rid of.

Not really, no. I just didn't want to introduce or remove any layers to make
this CL more readable. Basically, so that all replacements are context-free.

Do you mind if I leave this kind of cleanup for a follow-up CL?

https://codereview.chromium.org/965103004/diff/60001/chrome/browser/media/med...
File chrome/browser/media/media_stream_devices_controller.cc (right):

https://codereview.chromium.org/965103004/diff/60001/chrome/browser/media/med...
chrome/browser/media/media_stream_devices_controller.cc:595: ) ==
CONTENT_SETTING_BLOCK &&
On 2015/03/06 16:05:33, markusheintz_ wrote:
> The closing ) should be in the previous line. 
> 
> Here and below

Done.

https://codereview.chromium.org/965103004/diff/60001/components/content_setti...
File
components/content_settings/core/browser/content_settings_default_provider.cc
(right):

https://codereview.chromium.org/965103004/diff/60001/components/content_setti...
components/content_settings/core/browser/content_settings_default_provider.cc:354:
default_settings_[CONTENT_SETTINGS_TYPE_MEDIASTREAM]->GetAsInteger(
On 2015/03/06 16:05:33, markusheintz_ wrote:
> Do the default_settings_ contain values for settings that were never changed
by
> the user?

Hm, you're right. I was under incorrect impression that all values would be set.
It looks like they don't have to be. This can be handled the same way as on
lines 122-176 by using ValueToContentSetting, which interprets NULL as
CONTENT_SETTING_DEFAULT.

[msramek, 2015-03-06 17:54:59]

msramek@chromium.org changed reviewers:
+ bauerb@chromium.org, jochen@chromium.org

[msramek, 2015-03-06 17:54:59]

Hi Bernhard and Jochen,

could you please also have a look at this?

Thanks,
Martin

[jochen (gone - plz use gerrit), 2015-03-09 15:40:52]

Markus already started to do the review, no?

[Bernhard Bauer, 2015-03-09 15:45:46]

LGTM if Markus is happy.

https://codereview.chromium.org/965103004/diff/100001/chrome/browser/media/me...
File chrome/browser/media/media_capture_devices_dispatcher.cc (right):

https://codereview.chromium.org/965103004/diff/100001/chrome/browser/media/me...
chrome/browser/media/media_capture_devices_dispatcher.cc:406: profile,
security_origin, contentSettingsType))
Nit: If the condition spans multiple lines, it's usually more readable to wrap
the body in braces.

[msramek, 2015-03-09 16:20:46]

Jochen: Yes, and Markus also knows the whole context of this change. I added you
as an owner of chrome/browser/media/ though.

https://codereview.chromium.org/965103004/diff/100001/chrome/browser/media/me...
File chrome/browser/media/media_capture_devices_dispatcher.cc (right):

https://codereview.chromium.org/965103004/diff/100001/chrome/browser/media/me...
chrome/browser/media/media_capture_devices_dispatcher.cc:406: profile,
security_origin, contentSettingsType))
On 2015/03/09 15:45:46, Bernhard Bauer wrote:
> Nit: If the condition spans multiple lines, it's usually more readable to wrap
> the body in braces.

Done. Here and below.

[jochen (gone - plz use gerrit), 2015-03-09 16:22:31]

it's preferable to ask somebody from c/b/media/OWNERS to review this

[msramek, 2015-03-09 16:31:33]

Ah. Sorry, didn't notice that git cl owners offered you from a higher file.

[msramek, 2015-03-09 16:32:23]

msramek@chromium.org changed reviewers:
+ tommi@chromium.org
- jochen@chromium.org

[msramek, 2015-03-09 16:32:23]

tommi@, could you PTAL at this as a media/ owner?

[markusheintz_, 2015-03-10 21:51:41]

On 2015/03/09 16:32:23, msramek wrote:
> tommi@, could you PTAL at this as a media/ owner?

LGTM

[tommi (sloooow) - chröme, 2015-03-13 22:11:20]

The CQ bit was checked by tommi@chromium.org

[tommi (sloooow) - chröme, 2015-03-13 22:11:21]

lgtm

[tommi (sloooow) - chröme, 2015-03-13 22:11:21]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/965103004/#ps120001
(title: "Added braces.")

[commit-bot: I haz the power, 2015-03-13 22:11:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/965103004/120001

[commit-bot: I haz the power, 2015-03-13 22:15:10]

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

[commit-bot: I haz the power, 2015-03-13 22:15:11]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[msramek, 2015-03-25 10:45:08]

https://codereview.chromium.org/965103004/diff/140001/components/content_sett...
File
components/content_settings/core/browser/content_settings_default_provider.cc
(right):

https://codereview.chromium.org/965103004/diff/140001/components/content_sett...
components/content_settings/core/browser/content_settings_default_provider.cc:522:
return;
I rebased over 1004733003 and forgot to split the patchest, sorry.

The only change is here.
- We migrate the default media settings even if not in incognito (we're not
writing new data, just changing the structure; default settings are used even in
incognito, so we must make sure we use the new prefs already)
- Instead of SetWebsiteSetting, we simply copy the values of old preferences to
the new ones (and do this before calling ReadDefaultSettings).

[Bernhard Bauer, 2015-03-26 10:12:57]

Still LGTM

[msramek, 2015-03-26 11:22:29]

The CQ bit was checked by msramek@chromium.org

[msramek, 2015-03-26 11:22:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
markusheintz@chromium.org, tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/965103004/#ps140001
(title: "Rebase over 1004733003.")

[commit-bot: I haz the power, 2015-03-26 11:22:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/965103004/140001

[commit-bot: I haz the power, 2015-03-26 12:14:30]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2015-03-26 12:15:04]

Patchset 7 (id:??) landed as
https://crrev.com/f90963538b234e2bad01b2f182ba8d98b179b1a5
Cr-Commit-Position: refs/heads/master@{#322364}