Add UI to the Clear Browsing Data dialog to remove Flash LSO data.

Add UI to the Clear Browsing Data dialog to remove Flash LSO data.

As there is currently no Flash version supporting it yet, the checkbox should be disabled.

Screenshots: http://www.dropmocks.com/mHuJR (Note: also includes screenshots for the option in the content settings to clear data at shutdown, which isn't fully ready yet).

XIB changes: Add an additional checkbox to the dialog in ClearBrowsingData.xib, with the state attribute bound to |clearLSODataChecked| and the enabled attribute bound to |clearLSODataEnabled|.

BUG=58235
TEST=Go to Options > Under the Hood > Clear Browsing Data, check "Clear plug-in local storage and settings".

[Bernhard Bauer, 2010-12-21 23:32:59]

Hi, 

please review the following parts of this CL:

Peter: Views
Evan: DOM UI
Nico: Cocoa
Elliot: Gtk

Thanks!

[Nico, 2010-12-21 23:38:42]

The "Clear Browsing Data" dialog is the saddest dialog in all of chrome.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
File chrome/browser/ui/cocoa/clear_browsing_data_controller.h (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
chrome/browser/ui/cocoa/clear_browsing_data_controller.h:46:
PluginDataRemoverHelper clearLSODataEnabled_;
Style guide says: Only capitalize the first letter of an abbreviate (i.e.
|clearLsoDataEnabled|

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
chrome/browser/ui/cocoa/clear_browsing_data_controller.h:87: @property
(nonatomic) BOOL clearLSODataCheckbox;
|clearLsoDataChecked|?

[Bernhard Bauer, 2010-12-21 23:49:36]

On Tue, Dec 21, 2010 at 15:38,  <thakis@chromium.org> wrote:
> The "Clear Browsing Data" dialog is the saddest dialog in all of chrome.

http://goo.gl/2lcgr

>
http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
> File chrome/browser/ui/cocoa/clear_browsing_data_controller.h (right):
>
>
http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
> chrome/browser/ui/cocoa/clear_browsing_data_controller.h:46:
> PluginDataRemoverHelper clearLSODataEnabled_;
> Style guide says: Only capitalize the first letter of an abbreviate
> (i.e. |clearLsoDataEnabled|

Really? http://google-styleguide.googlecode.com/svn/trunk/objcguide.xml#Naming
says "Any class, category, method, or variable name may use all
capitals for initialisms within the name."

>
http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
> chrome/browser/ui/cocoa/clear_browsing_data_controller.h:87: @property
> (nonatomic) BOOL clearLSODataCheckbox;
> |clearLsoDataChecked|?
>
> http://codereview.chromium.org/5964008/
>

[Nico, 2010-12-21 23:59:07]

On Tue, Dec 21, 2010 at 3:49 PM, Bernhard Bauer <bauerb@chromium.org> wrote:
> On Tue, Dec 21, 2010 at 15:38,  <thakis@chromium.org> wrote:
>> The "Clear Browsing Data" dialog is the saddest dialog in all of chrome.
>
> http://goo.gl/2lcgr
>
>>
http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
>> File chrome/browser/ui/cocoa/clear_browsing_data_controller.h (right):
>>
>>
http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
>> chrome/browser/ui/cocoa/clear_browsing_data_controller.h:46:
>> PluginDataRemoverHelper clearLSODataEnabled_;
>> Style guide says: Only capitalize the first letter of an abbreviate
>> (i.e. |clearLsoDataEnabled|
>
> Really? http://google-styleguide.googlecode.com/svn/trunk/objcguide.xml#Naming
> says "Any class, category, method, or variable name may use all
> capitals for initialisms within the name."

O_O I thought everyone always makes fun of NSURL because it's the only
class name that's all caps – and we say that's ok?! Oh well, if the
guide says it's permitted and you feel like doing it, I guess I can't
stop you.

>
>>
http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
>> chrome/browser/ui/cocoa/clear_browsing_data_controller.h:87: @property
>> (nonatomic) BOOL clearLSODataCheckbox;
>> |clearLsoDataChecked|?
>>
>> http://codereview.chromium.org/5964008/
>>
>

[Bernhard Bauer, 2010-12-22 00:02:20]

On Tue, Dec 21, 2010 at 15:59, Nico Weber <thakis@chromium.org> wrote:
> On Tue, Dec 21, 2010 at 3:49 PM, Bernhard Bauer <bauerb@chromium.org> wrote:
>> On Tue, Dec 21, 2010 at 15:38,  <thakis@chromium.org> wrote:
>>> The "Clear Browsing Data" dialog is the saddest dialog in all of chrome.
>>
>> http://goo.gl/2lcgr
>>
>>>
http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
>>> File chrome/browser/ui/cocoa/clear_browsing_data_controller.h (right):
>>>
>>>
http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
>>> chrome/browser/ui/cocoa/clear_browsing_data_controller.h:46:
>>> PluginDataRemoverHelper clearLSODataEnabled_;
>>> Style guide says: Only capitalize the first letter of an abbreviate
>>> (i.e. |clearLsoDataEnabled|
>>
>> Really?
http://google-styleguide.googlecode.com/svn/trunk/objcguide.xml#Naming
>> says "Any class, category, method, or variable name may use all
>> capitals for initialisms within the name."
>
> O_O I thought everyone always makes fun of NSURL because it's the only
> class name that's all caps – and we say that's ok?! Oh well, if the
> guide says it's permitted and you feel like doing it, I guess I can't
> stop you.

I think it does look better than clearLsoDataEnabled, and it's more
consistent with the other occurrences.

>
>>
>>>
http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
>>> chrome/browser/ui/cocoa/clear_browsing_data_controller.h:87: @property
>>> (nonatomic) BOOL clearLSODataCheckbox;
>>> |clearLsoDataChecked|?
>>>
>>> http://codereview.chromium.org/5964008/
>>>
>>
>

[Peter Kasting, 2010-12-22 00:03:02]

views LGTM although I personally would prefer just combining this with the
cookies checkbox, since that already covers all other local storage.

I'll have that discussion separately with Glen et al.

[Evan Stade, 2010-12-22 00:16:39]

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/clear_browser_data_handler.cc (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.cc:111: break;
should unhandled PREF_CHANGED notifications be passed up to
OptionsPageUIHandler?

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.cc:157:
L"ClearBrowserDataOverlay.setClearPluginLSODataEnabled",
nit: no newline

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/clear_browser_data_handler.h (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.h:34: void
HandleClearBrowserData(const ListValue* value);
would you mind adding a comment here? it's hard at a glance to tell whether each
of these functions is handling or sending a DOMUI message

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.h:36: void
UpdateClearPluginLSOData();
comment

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.h:44:
PluginDataRemoverHelper clear_plugin_lso_data_enabled_;
comment, or at least vertically separate from |remover_|

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.js (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:58:
(function(self, pref) {
why is this a single use function and not just a block of code

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:64:
event.value['value'] : event.value;
as long as you're casting to boolean, can't you just do

event.value && event.value['value']

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:79:
self.addEventListener('click',
self.addEventListener('click', function (e) {
  [...]
});

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:87: f();
what

[Bernhard Bauer, 2010-12-22 00:19:35]

On Tue, Dec 21, 2010 at 16:03,  <pkasting@chromium.org> wrote:
> views LGTM although I personally would prefer just combining this with the
> cookies checkbox, since that already covers all other local storage.

I would like that as well, but the problem is that clearing plugin LSO
data has considerably more overhead (we have to start the plugin
process if it's not running, and do everything over IPC), and it also
has an effect on other browsers, so some user may not want to do it.
Also, if the user doesn't have a Flash version which implements the
NPP_ClearSiteData API we need to disable the feature, because we'd
otherwise create a false impression of privacy.

> I'll have that discussion separately with Glen et al.
>
> http://codereview.chromium.org/5964008/
>

[Peter Kasting, 2010-12-22 00:24:12]

On 2010/12/22 00:19:35, Bernhard Bauer wrote:
> I would like that as well, but the problem is that clearing plugin LSO
> data has considerably more overhead (we have to start the plugin
> process if it's not running, and do everything over IPC), and it also
> has an effect on other browsers, so some user may not want to do it.

The overhead doesn't seem like a big deal.  The having an effect on other
browsers part is legit, but it seems much more likely that users will want to
clear everything and fail to understand and check the Flash box than that they
will use another browser, understand that clearing Flash data will affect that
browser, and for that reason elect not to clear it here.

In other words, I think the benefit outweighs the cost.

> Also, if the user doesn't have a Flash version which implements the
> NPP_ClearSiteData API we need to disable the feature, because we'd
> otherwise create a false impression of privacy.

But we're shipping the Flash version, no?  Wouldn't this only affect users who
disable the built-in Flash and have an outdated external Flash?  That seems like
too small a group to reckon about.

[Bernhard Bauer, 2010-12-22 01:12:03]

On 2010/12/22 00:24:12, Peter Kasting wrote:
> On 2010/12/22 00:19:35, Bernhard Bauer wrote:
> > I would like that as well, but the problem is that clearing plugin LSO
> > data has considerably more overhead (we have to start the plugin
> > process if it's not running, and do everything over IPC), and it also
> > has an effect on other browsers, so some user may not want to do it.
> 
> The overhead doesn't seem like a big deal.  The having an effect on other
> browsers part is legit, but it seems much more likely that users will want to
> clear everything and fail to understand and check the Flash box than that they
> will use another browser, understand that clearing Flash data will affect that
> browser, and for that reason elect not to clear it here.
> 
> In other words, I think the benefit outweighs the cost.
> 
> > Also, if the user doesn't have a Flash version which implements the
> > NPP_ClearSiteData API we need to disable the feature, because we'd
> > otherwise create a false impression of privacy.
> 
> But we're shipping the Flash version, no?  Wouldn't this only affect users who
> disable the built-in Flash and have an outdated external Flash?  That seems
like
> too small a group to reckon about.

I'll reply on your separate thread.

[Bernhard Bauer, 2010-12-22 03:40:04]

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/clear_browser_data_handler.cc (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.cc:111: break;
On 2010/12/22 00:16:40, Evan Stade wrote:
> should unhandled PREF_CHANGED notifications be passed up to
> OptionsPageUIHandler?

Hm. I don't want to confuse OptionsPageUIHandler with preferences it didn't
register, on the other hand receiving a notification we didn't register for
would indeed warrant a DCHECK(). But the current implementation is empty anyway,
so I'd say we do it, just to be consistent with below.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.cc:157:
L"ClearBrowserDataOverlay.setClearPluginLSODataEnabled",
On 2010/12/22 00:16:40, Evan Stade wrote:
> nit: no newline

I think the rule is if you start breaking lines, you put each parameter on a
separate line.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/clear_browser_data_handler.h (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.h:34: void
HandleClearBrowserData(const ListValue* value);
On 2010/12/22 00:16:40, Evan Stade wrote:
> would you mind adding a comment here? it's hard at a glance to tell whether
each
> of these functions is handling or sending a DOMUI message

Done.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.h:36: void
UpdateClearPluginLSOData();
On 2010/12/22 00:16:40, Evan Stade wrote:
> comment

Done.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.h:44:
PluginDataRemoverHelper clear_plugin_lso_data_enabled_;
On 2010/12/22 00:16:40, Evan Stade wrote:
> comment, or at least vertically separate from |remover_|

Done.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.js (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:58:
(function(self, pref) {
On 2010/12/22 00:16:40, Evan Stade wrote:
> why is this a single use function and not just a block of code

To restrict the scope of |self| and |pref| to this function, so we don't get
potential naming conflicts (and still can use short variable names).

If you think it hinders readability though, I'll inline it.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:64:
event.value['value'] : event.value;
On 2010/12/22 00:16:40, Evan Stade wrote:
> as long as you're casting to boolean, can't you just do
> 
> event.value && event.value['value']

Probably. I have to admit, I copied that piece of code from Preferences.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:79:
self.addEventListener('click',
On 2010/12/22 00:16:40, Evan Stade wrote:
> self.addEventListener('click', function (e) {
>   [...]
> });

Done.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:87: f();
On 2010/12/22 00:16:40, Evan Stade wrote:
> what

|f| is the updateCommitButtonState_ function bound to |this| (i.e. the
ClearBrowserDataOverlay). I copied it from above, but admittedly it should have
a longer name if it's being used so far from its declaration.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
File chrome/browser/ui/cocoa/clear_browsing_data_controller.h (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/ui/cocoa/clea...
chrome/browser/ui/cocoa/clear_browsing_data_controller.h:87: @property
(nonatomic) BOOL clearLSODataCheckbox;
On 2010/12/21 23:38:42, Nico wrote:
> |clearLsoDataChecked|?

Makes sense (except for the capitalization part. That I will nevarrr accept!).

[Evan Stade, 2010-12-22 21:36:16]

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/clear_browser_data_handler.cc (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.cc:157:
L"ClearBrowserDataOverlay.setClearPluginLSODataEnabled",
On 2010/12/22 03:40:05, Bernhard Bauer wrote:
> On 2010/12/22 00:16:40, Evan Stade wrote:
> > nit: no newline
> 
> I think the rule is if you start breaking lines, you put each parameter on a
> separate line.

actually I think either way is acceptable, but this way seems less readable to
me (the same isn't true when there are many arguments. In that case, one arg per
line is easier to read)

but I don't feel strongly

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.js (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:58:
(function(self, pref) {
On 2010/12/22 03:40:05, Bernhard Bauer wrote:
> On 2010/12/22 00:16:40, Evan Stade wrote:
> > why is this a single use function and not just a block of code
> 
> To restrict the scope of |self| and |pref| to this function, so we don't get
> potential naming conflicts (and still can use short variable names).
> 
> If you think it hinders readability though, I'll inline it.

yes please

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:87: f();
On 2010/12/22 03:40:05, Bernhard Bauer wrote:
> On 2010/12/22 00:16:40, Evan Stade wrote:
> > what
> 
> |f| is the updateCommitButtonState_ function bound to |this| (i.e. the
> ClearBrowserDataOverlay). I copied it from above, but admittedly it should
have
> a longer name if it's being used so far from its declaration.

oh, I see. Yes, it should have a better name.

[Peter Kasting, 2010-12-22 22:39:21]

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/clear_browser_data_handler.cc (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.cc:157:
L"ClearBrowserDataOverlay.setClearPluginLSODataEnabled",
On 2010/12/22 03:40:05, Bernhard Bauer wrote:
> I think the rule is if you start breaking lines, you put each parameter on a
> separate line.

That's true of function declarations and definitions, but at callsites you're
allowed to do whatever is most readable.

[Bernhard Bauer, 2011-01-03 11:56:29]

I think that's everything in DOM UI. PTAL (also Nico, Elliot)?

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
File chrome/browser/dom_ui/options/clear_browser_data_handler.cc (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/dom_ui/option...
chrome/browser/dom_ui/options/clear_browser_data_handler.cc:157:
L"ClearBrowserDataOverlay.setClearPluginLSODataEnabled",
On 2010/12/22 22:39:22, Peter Kasting wrote:
> On 2010/12/22 03:40:05, Bernhard Bauer wrote:
> > I think the rule is if you start breaking lines, you put each parameter on a
> > separate line.
> 
> That's true of function declarations and definitions, but at callsites you're
> allowed to do whatever is most readable.

OK, Done.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.js (right):

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:58:
(function(self, pref) {
On 2010/12/22 21:36:16, Evan Stade wrote:
> On 2010/12/22 03:40:05, Bernhard Bauer wrote:
> > On 2010/12/22 00:16:40, Evan Stade wrote:
> > > why is this a single use function and not just a block of code
> > 
> > To restrict the scope of |self| and |pref| to this function, so we don't get
> > potential naming conflicts (and still can use short variable names).
> > 
> > If you think it hinders readability though, I'll inline it.
> 
> yes please

Done.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:64:
event.value['value'] : event.value;
On 2010/12/22 00:16:40, Evan Stade wrote:
> as long as you're casting to boolean, can't you just do
> 
> event.value && event.value['value']

Done.

http://codereview.chromium.org/5964008/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.js:87: f();
On 2010/12/22 21:36:16, Evan Stade wrote:
> On 2010/12/22 03:40:05, Bernhard Bauer wrote:
> > On 2010/12/22 00:16:40, Evan Stade wrote:
> > > what
> > 
> > |f| is the updateCommitButtonState_ function bound to |this| (i.e. the
> > ClearBrowserDataOverlay). I copied it from above, but admittedly it should
> have
> > a longer name if it's being used so far from its declaration.
> 
> oh, I see. Yes, it should have a better name.

Done.

[Nico, 2011-01-03 16:11:26]

LG

[Elliot Glaysher, 2011-01-04 22:38:22]

gtk parts lgtm

On Mon, Jan 3, 2011 at 8:11 AM,  <thakis@chromium.org> wrote:
> LG
>
> http://codereview.chromium.org/5964008/
>

[Evan Stade, 2011-01-04 22:59:24]

http://codereview.chromium.org/5964008/diff/27001/chrome/browser/resources/op...
File chrome/browser/resources/options/clear_browser_data_overlay.js (right):

http://codereview.chromium.org/5964008/diff/27001/chrome/browser/resources/op...
chrome/browser/resources/options/clear_browser_data_overlay.js:66:
event.value['managed'];
can this not fit on the above line? either way, the indent here is strange

http://codereview.chromium.org/5964008/diff/27001/chrome/browser/resources/op...
chrome/browser/resources/options/clear_browser_data_overlay.js:71:
function(enabled) {
this can fit on the above line I believe

http://codereview.chromium.org/5964008/diff/27001/chrome/browser/resources/op...
chrome/browser/resources/options/clear_browser_data_overlay.js:77:
checkbox.addEventListener('click', function(e) {
why is it insufficient to use a pref attribute like the other checkboxes?

[Bernhard Bauer, 2011-01-04 23:15:32]

http://codereview.chromium.org/5964008/diff/27001/chrome/browser/resources/op...
File chrome/browser/resources/options/clear_browser_data_overlay.js (right):

http://codereview.chromium.org/5964008/diff/27001/chrome/browser/resources/op...
chrome/browser/resources/options/clear_browser_data_overlay.js:66:
event.value['managed'];
On 2011/01/04 22:59:24, Evan Stade wrote:
> can this not fit on the above line? either way, the indent here is strange

Done.

http://codereview.chromium.org/5964008/diff/27001/chrome/browser/resources/op...
chrome/browser/resources/options/clear_browser_data_overlay.js:71:
function(enabled) {
On 2011/01/04 22:59:24, Evan Stade wrote:
> this can fit on the above line I believe

Done.

http://codereview.chromium.org/5964008/diff/27001/chrome/browser/resources/op...
chrome/browser/resources/options/clear_browser_data_overlay.js:77:
checkbox.addEventListener('click', function(e) {
On 2011/01/04 22:59:24, Evan Stade wrote:
> why is it insufficient to use a pref attribute like the other checkboxes?

The checkbox is only checked if the preference is set *and* the user has the
correct Flash version installed (which is plumbed through to the UI via
setClearPluginLSODataEnabled), so we need to keep track of the UI state
separately from the preference in order to restore it to the correct value when
the right Flash version is enabled/installed.

[Bernhard Bauer, 2011-01-07 00:26:58]

Evan, ping?

[Bernhard Bauer, 2011-01-10 22:03:00]

https://memegen.googleplex.com/295124

On Fri, Jan 7, 2011 at 01:26,  <bauerb@chromium.org> wrote:
> Evan, ping?
>
> http://codereview.chromium.org/5964008/
>