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

Issue 27635002: Content settings for <audio> and <video>.

Created:
7 years, 2 months ago by pwnall-personal
Modified:
6 years, 5 months ago
Reviewers:
Bernhard Bauer, eseidel
CC:
chromium-reviews, dbeam+watch-options_chromium.org, extensions-reviews_chromium.org, anantha, arv+watch_chromium.org, dyu1, dennis_jeffrey, chromium-apps-reviews_chromium.org, markusheintz_
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

This change adds a content setting for blocking HTML5 media (<audio>, <video>). The setting is modeled after the image blocking setting. The setting is exposed in chrome://settings/content#contentSettings and as the chrome.contentSettings.media extension API. The icons used to indicate allowed / blocked HTML5 media in the address bar are temporary (red-tinted versions of the image icons) and intended to facilitate manual testing for the feature. They should be replaced with better-designed icons before M38 comes out. BUG=50132

Patch Set 1 : Retrying upload. #

Patch Set 2 : Test fix. #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+469 lines, -39 lines) Patch
M chrome/app/generated_resources.grd View 3 chunks +27 lines, -0 lines 2 comments Download
A + chrome/app/theme/default_100_percent/common/allowed_media.png View Binary file 0 comments Download
A + chrome/app/theme/default_100_percent/common/blocked_media.png View Binary file 0 comments Download
A chrome/app/theme/default_200_percent/common/allowed_media.png View Binary file 0 comments Download
A chrome/app/theme/default_200_percent/common/blocked_media.png View Binary file 0 comments Download
M chrome/app/theme/theme_resources.grd View 1 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/content_settings/content_settings_default_provider.cc View 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/content_settings/content_settings_policy_provider.cc View 8 chunks +24 lines, -0 lines 1 comment Download
M chrome/browser/content_settings/content_settings_utils.cc View 1 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/content_settings/host_content_settings_map_unittest.cc View 4 chunks +45 lines, -0 lines 0 comments Download
M chrome/browser/content_settings/tab_specific_content_settings.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/content_settings/tab_specific_content_settings_unittest.cc View 6 chunks +14 lines, -0 lines 0 comments Download
M chrome/browser/extensions/api/content_settings/content_settings_apitest.cc View 3 chunks +11 lines, -0 lines 0 comments Download
M chrome/browser/extensions/api/content_settings/content_settings_helpers.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/prefs/pref_functional_browsertest.cc View 1 4 chunks +51 lines, -5 lines 1 comment Download
M chrome/browser/resources/options/content_settings.html View 1 chunk +32 lines, -0 lines 0 comments Download
M chrome/browser/resources/options/content_settings_exceptions_area.html View 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/content_settings/content_setting_bubble_model.cc View 4 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc View 1 chunk +18 lines, -0 lines 0 comments Download
M chrome/browser/ui/content_settings/content_setting_image_model.cc View 3 chunks +5 lines, -3 lines 0 comments Download
M chrome/browser/ui/website_settings/website_settings.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/ui/website_settings/website_settings_ui.cc View 2 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/options/content_settings_handler.cc View 4 chunks +12 lines, -0 lines 0 comments Download
M chrome/common/content_settings.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/content_settings_types.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/extensions/api/content_settings.json View 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/common/extensions/docs/examples/api/contentSettings/popup.html View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/common/extensions/docs/examples/api/contentSettings/popup.js View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/common/pref_names.h View 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/common/render_messages.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/renderer/content_settings_observer.h View 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/renderer/content_settings_observer.cc View 1 chunk +20 lines, -0 lines 0 comments Download
M chrome/renderer/content_settings_observer_browsertest.cc View 4 chunks +106 lines, -4 lines 0 comments Download
M chrome/test/data/extensions/api_test/content_settings/getresourceidentifiers/test.js View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/data/extensions/api_test/content_settings/standard/test.js View 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/test/data/policy/policy_test_cases.json View 1 1 chunk +18 lines, -0 lines 0 comments Download
A + chrome/test/data/settings/video_page.html View 1 1 chunk +13 lines, -21 lines 0 comments Download
M tools/metrics/actions/actions.xml View 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
eseidel
Did you mean to send this for review? (rietveld doesn't mail when you upload)
6 years, 8 months ago (2014-04-10 05:28:10 UTC) #1
pwnall-personal
On 2014/04/10 05:28:10, eseidel wrote: > Did you mean to send this for review? (rietveld ...
6 years, 8 months ago (2014-04-10 11:25:07 UTC) #2
pwnall-personal
I think I got this rebased, but I can't use the trybots because git-cl is ...
6 years, 6 months ago (2014-06-14 05:23:16 UTC) #3
Bernhard Bauer
6 years, 5 months ago (2014-07-01 09:12:50 UTC) #4
Looks pretty good in general! I agree that we should get nicer images though,
and we'll also want to get sign-off on the strings.

https://codereview.chromium.org/27635002/diff/852001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/27635002/diff/852001/chrome/app/generated_res...
chrome/app/generated_resources.grd:354: +        Sounds and videos
We usually use the term "audio" instead of "sounds".

https://codereview.chromium.org/27635002/diff/852001/chrome/app/generated_res...
chrome/app/generated_resources.grd:7737: +        Media
We should probably decide whether we want to use the "media" or "audio/video".

https://codereview.chromium.org/27635002/diff/852001/chrome/browser/content_s...
File chrome/browser/content_settings/content_settings_policy_provider.cc
(right):

https://codereview.chromium.org/27635002/diff/852001/chrome/browser/content_s...
chrome/browser/content_settings/content_settings_policy_provider.cc:32:
prefs::kManagedDefaultMediaSetting,
Not all of these are immediately necessary. The changes in this file for example
are if we want enterprise policy support for these content settings (which is
something we don't have for all content settings). It might make sense to pull
these out into a separate CL that someone from the enterprise team can then look
at.

https://codereview.chromium.org/27635002/diff/852001/chrome/browser/prefs/pre...
File chrome/browser/prefs/pref_functional_browsertest.cc (right):

https://codereview.chromium.org/27635002/diff/852001/chrome/browser/prefs/pre...
chrome/browser/prefs/pref_functional_browsertest.cc:131:
EXPECT_TRUE(browser()->profile()->GetPrefs()->GetBoolean(
I would probably use ASSERT_TRUE, because if Javascript is not enabled, the rest
of the test will fail anyway.

Powered by Google App Engine
This is Rietveld 408576698