Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(142)

Issue 10537099: add "always allow" option to the mediastream infobar and allow user to allow/not allow acces to devi (Closed)

Created:
8 years, 6 months ago by no longer working on chromium
Modified:
8 years, 6 months ago
CC:
chromium-reviews, dyu1, Nirnimesh, kkania, feature-media-reviews_chromium.org, anantha, robertshield, arv (Not doing code reviews), dennis_jeffrey, pam+watch_chromium.org
Visibility:
Public.

Description

Feature1: User goes to http://neave.com/webcam/html5/ The url displayed is the security origin, just like maps.google.com User needs to allow or deny access to his camera and microphone. Some sites will require only the mic or only the webcam. User can open an options drop down. The drop down needs to be clicked for every modification. A divide line separates microphones and webcams. User can also decide to Always allow. The decision will also remember the devices chosen. Details: The "Always allow" option will remember both the decision and the devices chosen. If "Always allow" is selected and Deny is pressed, access is denied. Upon the next visit, the infobar will be displayed again. If a a website is Allowed and "Always allowed": ...And devices disappears from the computer. It will show the infobar and ask the user to choose devices. Feature2: Settings => Privacy => Content Settings We want to add a new section called Media to cover the getUserMedia decisions. And we are able to see and remove the exception lists there. BUG=132075, 122764 TEST=go to https://apprtc.appspot.com; click "always allow this site to use this device" option; go to content setting/privacy/media, try ask/block settings and remove exceptions. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=143918

Patch Set 1 #

Total comments: 34

Patch Set 2 : use only content settings, added a device controller, addressed all the comments #

Total comments: 44

Patch Set 3 : rebased, addressed comments from Ivan, let "block" apply before exceptions #

Total comments: 8

Patch Set 4 : changed the i18n var names to be camelcase. #

Patch Set 5 : Check the exceptions before the default setting, sites in the exceptions list can access the device… #

Total comments: 14

Patch Set 6 : addressed all the comments and add support to Incognito mode. #

Patch Set 7 : addressed comments from joaodasilva and bauerb, added support to Incognito mode. #

Total comments: 12

Patch Set 8 : rebased && fixed small problems found by trybots #

Patch Set 9 : rebased #

Patch Set 10 : rebased #

Total comments: 1

Patch Set 11 : addressed sky's comment and replaced "Do not allow any site to" with "Do not allow sites to" #

Unified diffs Side-by-side diffs Delta from patch set Stats (+851 lines, -267 lines) Patch
M chrome/app/chrome_command_ids.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/app/generated_resources.grd View 3 chunks +24 lines, -3 lines 0 comments Download
M chrome/app/policy/policy_templates.json View 1 2 3 4 5 6 7 2 chunks +25 lines, -1 line 0 comments Download
M chrome/browser/automation/testing_automation_provider.cc View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/content_settings/content_settings_default_provider.cc View 1 2 3 4 5 6 7 8 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/content_settings/content_settings_policy_provider.cc View 1 4 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/content_settings/content_settings_pref_provider.cc View 1 2 3 4 5 6 2 chunks +24 lines, -7 lines 0 comments Download
M chrome/browser/content_settings/content_settings_utils.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/content_settings/host_content_settings_map.h View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/content_settings/host_content_settings_map.cc View 1 2 3 4 5 6 7 8 6 chunks +25 lines, -12 lines 0 comments Download
A chrome/browser/media/media_stream_devices_controller.h View 1 2 3 4 5 6 7 1 chunk +87 lines, -0 lines 0 comments Download
A chrome/browser/media/media_stream_devices_controller.cc View 1 2 3 4 5 6 1 chunk +294 lines, -0 lines 0 comments Download
M chrome/browser/media/media_stream_devices_menu_model.h View 1 2 3 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/media/media_stream_devices_menu_model.cc View 1 2 3 4 5 6 5 chunks +43 lines, -15 lines 0 comments Download
M chrome/browser/policy/configuration_policy_handler_list.cc View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/resources/options2/content_settings.html View 1 2 3 1 chunk +20 lines, -0 lines 0 comments Download
M chrome/browser/resources/options2/content_settings_exceptions_area.html View 1 2 3 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/resources/options2/content_settings_exceptions_area.js View 1 2 3 4 5 18 chunks +31 lines, -12 lines 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 4 5 6 7 8 9 2 chunks +21 lines, -13 lines 0 comments Download
M chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa_unittest.mm View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/infobars/media_stream_infobar_controller.mm View 1 2 3 4 5 6 2 chunks +8 lines, -7 lines 0 comments Download
M chrome/browser/ui/gtk/infobars/media_stream_infobar_gtk.cc View 1 2 2 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/ui/media_stream_infobar_delegate.h View 1 2 3 4 5 6 7 8 9 10 4 chunks +12 lines, -26 lines 0 comments Download
M chrome/browser/ui/media_stream_infobar_delegate.cc View 1 2 3 4 5 6 2 chunks +20 lines, -77 lines 0 comments Download
M chrome/browser/ui/views/infobars/media_stream_infobar.cc View 1 2 3 4 5 6 7 2 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/ui/webui/options2/content_settings_handler2.h View 1 2 3 4 5 6 2 chunks +13 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options2/content_settings_handler2.cc View 1 2 3 4 5 6 7 8 9 chunks +140 lines, -79 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/content_settings_types.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -1 line 0 comments Download
M chrome/common/pref_names.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/test/functional/policy_test_cases.py View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/tools/chromeactions.txt View 1 2 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 33 (0 generated)
no longer working on chromium
Hi all, Would you please help reviewing this CL? This is a key feature webrtc ...
8 years, 6 months ago (2012-06-11 15:51:43 UTC) #1
Bernhard Bauer
It feels a bit weird that we store origins that are allowed to access media ...
8 years, 6 months ago (2012-06-11 18:16:00 UTC) #2
no longer working on chromium
Hi Andrew, We are trying to push this new UI feature out for M21, which ...
8 years, 6 months ago (2012-06-11 19:13:30 UTC) #3
tommi (sloooow) - chröme
good job xians - I briefly looked of the cl and agree with the comments ...
8 years, 6 months ago (2012-06-11 20:59:21 UTC) #4
Evan Stade
this is kind of an "innovative" use of a <select> dropdown. Correct me if I'm ...
8 years, 6 months ago (2012-06-12 01:51:12 UTC) #5
jochen (gone - plz use gerrit)
I defer my part to Bernhard
8 years, 6 months ago (2012-06-12 15:01:29 UTC) #6
no longer working on chromium
Sorry for the delay on uploading a new version, we just had a day-trip summer ...
8 years, 6 months ago (2012-06-14 13:03:25 UTC) #7
no longer working on chromium
On 2012/06/12 01:51:12, Evan Stade wrote: > this is kind of an "innovative" use of ...
8 years, 6 months ago (2012-06-14 15:25:14 UTC) #8
Ivan Korotkov
I agree with Evan that "always allows" as a select option is pretty confusing. Why ...
8 years, 6 months ago (2012-06-14 17:14:12 UTC) #9
no longer working on chromium
To Ivan and Evan, The "always allow" is "always allow this site to use this ...
8 years, 6 months ago (2012-06-15 16:52:06 UTC) #10
Evan Stade
http://codereview.chromium.org/10537099/diff/16001/chrome/browser/resources/options2/content_settings.html File chrome/browser/resources/options2/content_settings.html (right): http://codereview.chromium.org/10537099/diff/16001/chrome/browser/resources/options2/content_settings.html#newcode301 chrome/browser/resources/options2/content_settings.html:301: <h3 i18n-content="mediastream_tab_label"></h3> these i18n var names should be camel ...
8 years, 6 months ago (2012-06-15 18:51:25 UTC) #11
no longer working on chromium
http://codereview.chromium.org/10537099/diff/16001/chrome/browser/resources/options2/content_settings.html File chrome/browser/resources/options2/content_settings.html (right): http://codereview.chromium.org/10537099/diff/16001/chrome/browser/resources/options2/content_settings.html#newcode301 chrome/browser/resources/options2/content_settings.html:301: <h3 i18n-content="mediastream_tab_label"></h3> On 2012/06/15 18:51:25, Evan Stade wrote: > ...
8 years, 6 months ago (2012-06-15 21:04:47 UTC) #12
Evan Stade
http://codereview.chromium.org/10537099/diff/16001/chrome/browser/resources/options2/content_settings.html File chrome/browser/resources/options2/content_settings.html (right): http://codereview.chromium.org/10537099/diff/16001/chrome/browser/resources/options2/content_settings.html#newcode301 chrome/browser/resources/options2/content_settings.html:301: <h3 i18n-content="mediastream_tab_label"></h3> On 2012/06/15 21:04:47, xians1 wrote: > On ...
8 years, 6 months ago (2012-06-15 21:33:32 UTC) #13
no longer working on chromium
https://chromiumcodereview.appspot.com/10537099/diff/16001/chrome/browser/resources/options2/content_settings.html File chrome/browser/resources/options2/content_settings.html (right): https://chromiumcodereview.appspot.com/10537099/diff/16001/chrome/browser/resources/options2/content_settings.html#newcode301 chrome/browser/resources/options2/content_settings.html:301: <h3 i18n-content="mediastream_tab_label"></h3> On 2012/06/15 21:33:32, Evan Stade wrote: > ...
8 years, 6 months ago (2012-06-18 10:22:49 UTC) #14
no longer working on chromium
After discussing with the privacy team and Bernhard, I changed the behavior to: If a ...
8 years, 6 months ago (2012-06-18 16:43:19 UTC) #15
Bernhard Bauer
https://chromiumcodereview.appspot.com/10537099/diff/8004/chrome/browser/content_settings/host_content_settings_map.cc File chrome/browser/content_settings/host_content_settings_map.cc (left): https://chromiumcodereview.appspot.com/10537099/diff/8004/chrome/browser/content_settings/host_content_settings_map.cc#oldcode188 chrome/browser/content_settings/host_content_settings_map.cc:188: DCHECK(!ContentTypeHasCompoundValue(content_type)); Are you sure about this? This method is ...
8 years, 6 months ago (2012-06-18 17:38:14 UTC) #16
Evan Stade
my part lgtm https://chromiumcodereview.appspot.com/10537099/diff/24004/chrome/browser/resources/options2/content_settings_exceptions_area.js File chrome/browser/resources/options2/content_settings_exceptions_area.js (right): https://chromiumcodereview.appspot.com/10537099/diff/24004/chrome/browser/resources/options2/content_settings_exceptions_area.js#newcode16 chrome/browser/resources/options2/content_settings_exceptions_area.js:16: * in the select. I believe ...
8 years, 6 months ago (2012-06-18 19:12:58 UTC) #17
Ivan Korotkov
My comments and OOBE perspective LGTM
8 years, 6 months ago (2012-06-19 09:22:04 UTC) #18
no longer working on chromium
https://chromiumcodereview.appspot.com/10537099/diff/8004/chrome/browser/content_settings/host_content_settings_map.cc File chrome/browser/content_settings/host_content_settings_map.cc (left): https://chromiumcodereview.appspot.com/10537099/diff/8004/chrome/browser/content_settings/host_content_settings_map.cc#oldcode188 chrome/browser/content_settings/host_content_settings_map.cc:188: DCHECK(!ContentTypeHasCompoundValue(content_type)); On 2012/06/18 17:38:14, Bernhard Bauer wrote: > Are ...
8 years, 6 months ago (2012-06-19 12:23:16 UTC) #19
Joao da Silva
The enterprise team was at a summit last week, sorry for the late review. This ...
8 years, 6 months ago (2012-06-19 14:28:08 UTC) #20
Bernhard Bauer
https://chromiumcodereview.appspot.com/10537099/diff/8004/chrome/browser/content_settings/host_content_settings_map.cc File chrome/browser/content_settings/host_content_settings_map.cc (left): https://chromiumcodereview.appspot.com/10537099/diff/8004/chrome/browser/content_settings/host_content_settings_map.cc#oldcode188 chrome/browser/content_settings/host_content_settings_map.cc:188: DCHECK(!ContentTypeHasCompoundValue(content_type)); On 2012/06/19 12:23:16, xians1 wrote: > On 2012/06/18 ...
8 years, 6 months ago (2012-06-19 14:33:34 UTC) #21
no longer working on chromium
Hello, Sorry for the delay on uploading a new version, we stockholm just had a ...
8 years, 6 months ago (2012-06-20 17:24:38 UTC) #22
Bernhard Bauer
https://chromiumcodereview.appspot.com/10537099/diff/34036/chrome/browser/content_settings/content_settings_pref_provider.cc File chrome/browser/content_settings/content_settings_pref_provider.cc (right): https://chromiumcodereview.appspot.com/10537099/diff/34036/chrome/browser/content_settings/content_settings_pref_provider.cc#newcode371 chrome/browser/content_settings/content_settings_pref_provider.cc:371: DCHECK(!setting->empty()); On 2012/06/20 17:24:38, xians1 wrote: > To bauerb: ...
8 years, 6 months ago (2012-06-20 18:33:27 UTC) #23
no longer working on chromium
On 2012/06/20 18:33:27, Bernhard Bauer wrote: > https://chromiumcodereview.appspot.com/10537099/diff/34036/chrome/browser/content_settings/content_settings_pref_provider.cc > File chrome/browser/content_settings/content_settings_pref_provider.cc (right): > > https://chromiumcodereview.appspot.com/10537099/diff/34036/chrome/browser/content_settings/content_settings_pref_provider.cc#newcode371 ...
8 years, 6 months ago (2012-06-21 19:46:17 UTC) #24
Bernhard Bauer
LGTM
8 years, 6 months ago (2012-06-22 08:16:39 UTC) #25
Joao da Silva
lgtm
8 years, 6 months ago (2012-06-22 16:58:04 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xians@chromium.org/10537099/61002
8 years, 6 months ago (2012-06-25 12:44:53 UTC) #27
commit-bot: I haz the power
Failed to apply patch for chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa_unittest.mm: While running patch -p1 --forward --force; patching file chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa_unittest.mm ...
8 years, 6 months ago (2012-06-25 12:44:59 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xians@chromium.org/10537099/64002
8 years, 6 months ago (2012-06-25 14:13:30 UTC) #29
commit-bot: I haz the power
Presubmit check for 10537099-64002 failed and returned exit status 1. Running presubmit commit checks ...
8 years, 6 months ago (2012-06-25 14:13:40 UTC) #30
sky
LGTM https://chromiumcodereview.appspot.com/10537099/diff/64002/chrome/browser/ui/media_stream_infobar_delegate.h File chrome/browser/ui/media_stream_infobar_delegate.h (right): https://chromiumcodereview.appspot.com/10537099/diff/64002/chrome/browser/ui/media_stream_infobar_delegate.h#newcode23 chrome/browser/ui/media_stream_infobar_delegate.h:23: MediaStreamDevicesController* controller); Document this takes ownership of |controller|.
8 years, 6 months ago (2012-06-25 14:19:42 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xians@chromium.org/10537099/65005
8 years, 6 months ago (2012-06-25 14:29:08 UTC) #32
commit-bot: I haz the power
8 years, 6 months ago (2012-06-25 17:00:58 UTC) #33
Change committed as 143918

Powered by Google App Engine
This is Rietveld 408576698