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

Issue 4643007: Move click-to-play to about:flags. (Closed)

Created:
10 years, 1 month ago by Bernhard Bauer
Modified:
9 years, 7 months ago
CC:
chromium-reviews, darin-cc_chromium.org, brettw-cc_chromium.org, pam+watch_chromium.org, ben+cc_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

Move click-to-play to about:flags. XIB changes: Add an outlet |pluginDefaultSettingMatrix_| to ContentSettingsDialogController, hooked up to the associated matrix, to remove the click-to-play radio button. While I'm at it, clean up a bit: * Remove the old --disable-click-to-play flag that reverted to the M6 behavior for blocked plugins * Make ContentExceptionsWindowController use ContentSettingComboModel for the action popup. * Make HostContentSettingsMapTest use AutoReset to reset command line switches. BUG=62091 TEST=unit_tests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=65953

Patch Set 1 #

Patch Set 2 : fix #

Patch Set 3 : fix unit test #

Patch Set 4 : delete moar code #

Patch Set 5 : sync #

Patch Set 6 : test #

Patch Set 7 : fix tests #

Total comments: 2

Patch Set 8 : review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+198 lines, -172 lines) Patch
M chrome/app/generated_resources.grd View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/app/nibs/ContentSettings.xib View 4 chunks +27 lines, -21 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/browser/cocoa/content_exceptions_window_controller.h View 2 chunks +2 lines, -3 lines 0 comments Download
M chrome/browser/cocoa/content_exceptions_window_controller.mm View 8 chunks +11 lines, -63 lines 0 comments Download
M chrome/browser/cocoa/content_setting_bubble_cocoa.mm View 1 chunk +11 lines, -26 lines 0 comments Download
M chrome/browser/cocoa/content_settings_dialog_controller.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/cocoa/content_settings_dialog_controller.mm View 2 chunks +27 lines, -3 lines 0 comments Download
M chrome/browser/cocoa/content_settings_dialog_controller_unittest.mm View 3 4 chunks +23 lines, -7 lines 0 comments Download
M chrome/browser/content_setting_bubble_model.cc View 1 chunk +2 lines, -7 lines 0 comments Download
M chrome/browser/content_setting_bubble_model_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/content_setting_combo_model.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/gtk/options/content_filter_page_gtk.cc View 1 2 3 1 chunk +9 lines, -2 lines 0 comments Download
M chrome/browser/host_content_settings_map.cc View 4 5 6 7 5 chunks +28 lines, -1 line 0 comments Download
M chrome/browser/host_content_settings_map_unittest.cc View 6 7 9 chunks +25 lines, -22 lines 0 comments Download
M chrome/browser/renderer_host/browser_render_process_host.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/options/content_filter_page_view.cc View 1 chunk +8 lines, -4 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 2 chunks +1 line, -1 line 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/renderer/render_view.cc View 1 2 3 4 1 chunk +1 line, -6 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Bernhard Bauer
Please review.
10 years, 1 month ago (2010-11-11 21:12:17 UTC) #1
Mattias Nissler (ping if slow)
I did a quick scan and have a single nit suggestion. Didn't do a thorough ...
10 years, 1 month ago (2010-11-12 11:49:46 UTC) #2
jochen (gone - plz use gerrit)
lgtm please include a description of the xib changes you might want to consider to ...
10 years, 1 month ago (2010-11-12 13:38:04 UTC) #3
Bernhard Bauer
10 years, 1 month ago (2010-11-12 15:08:07 UTC) #4
On 2010/11/12 11:49:46, Mattias Nissler wrote:
> I did a quick scan and have a single nit suggestion. Didn't do a thorough
review
> though.
> 
>
http://codereview.chromium.org/4643007/diff/16001/chrome/browser/host_content...
> File chrome/browser/host_content_settings_map.h (right):
> 
>
http://codereview.chromium.org/4643007/diff/16001/chrome/browser/host_content...
> chrome/browser/host_content_settings_map.h:253: ContentSetting
> ClickToPlayFixup(ContentSettingsType content_type,
> Maybe add a comment explaining what it does?

Done, and moved it to .cc file to reduce clutter.

On 2010/11/12 13:38:04, jochen wrote:
> lgtm
> 
> please include a description of the xib changes
> 
> you might want to consider to move the cleanup work into a separate CL for
> easier merging around

Meh. The point of making ContentExceptionsWindowController use
ContentSettingComboModel was because I was too lazy to change the settings array
there as well, so if I split it up, I'd have to merge both changes.

>
http://codereview.chromium.org/4643007/diff/16001/chrome/browser/host_content...
> File chrome/browser/host_content_settings_map_unittest.cc (right):
> 
>
http://codereview.chromium.org/4643007/diff/16001/chrome/browser/host_content...
> chrome/browser/host_content_settings_map_unittest.cc:96: CommandLine* cmd =
> CommandLine::ForCurrentProcess();
> can you make the other unit tests use this pattern as well?

done.

Powered by Google App Engine
This is Rietveld 408576698