Plugins page in BWSI session has unnecessary and misleading UI components

Plugins page in BWSI session has unnecessary and misleading UI components

Moved part of code from Options page to upper level so that it can be reused in Plugins page.

BUG=chromium-os:24947
TEST=Tested manually


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=132177

[Denis Kuznetsov (DE-MUC), 2012-04-06 10:09:48]

Nikita, please look at chrome/browser/ui/webui/chromeos/ part.
James, please look  on the rest.

[Nikita (slow), 2012-04-06 12:36:09]

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/plugi...
File chrome/browser/resources/plugins.html (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/plugi...
chrome/browser/resources/plugins.html:6: <link rel="stylesheet"
href="chrome://resources/css/chrome_shared2.css">
I don't see a new code in chrome_shared2.css for this patchset. Is this change
for bringing chrome://plugins page style closer to other chrome:// pages style?

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
File chrome/browser/resources/shared/css/widgets.css (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
chrome/browser/resources/shared/css/widgets.css:220: <if
expr="pp_ifdef('chromeos')">
chrome_shared2.css seems to be a better place for these rules.

As this file: "This file defines styles for form controls."
chrome_shared2.css: "This file holds CSS that should be shared, in theory, by
all user-visible chrome:// pages. This contrasts chrome_shared.css, which it is
meant to replace, but has CSS specific to options and options-like pages."

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
File chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:1: // Copyright
(c) 2012 The Chromium Authors. All rights reserved.
Please do a git cp
chrome/browser/resources/options2/chromeos/accounts_options.js <this file> to
minimize diff.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chromeos/ui_account_tweaks.cc (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.cc:7: #include "base/bind.h"
Please cleanup headers. You don't need bind related headers, authenticator,
policy etc.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.cc:27: * Fills given
dictionary with account status data (whether current user is
This comment should be moved to header.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.cc:51: * Fills given data
source with account status data (whether current user is
Remove / move to header.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chromeos/ui_account_tweaks.h (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:17: base::DictionaryValue*
localized_strings);
nit: align

[Denis Kuznetsov (DE-MUC), 2012-04-09 11:21:30]

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/plugi...
File chrome/browser/resources/plugins.html (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/plugi...
chrome/browser/resources/plugins.html:6: <link rel="stylesheet"
href="chrome://resources/css/chrome_shared2.css">
On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> I don't see a new code in chrome_shared2.css for this patchset. Is this change
> for bringing chrome://plugins page style closer to other chrome:// pages
style?
chrome_shared2.css includes widgets.css that are altered by this CL.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
File chrome/browser/resources/shared/css/widgets.css (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
chrome/browser/resources/shared/css/widgets.css:220: <if
expr="pp_ifdef('chromeos')">
On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> chrome_shared2.css seems to be a better place for these rules.
> 
> As this file: "This file defines styles for form controls."
> chrome_shared2.css: "This file holds CSS that should be shared, in theory, by
> all user-visible chrome:// pages. This contrasts chrome_shared.css, which it
is
> meant to replace, but has CSS specific to options and options-like pages."

On the other hand, widgets.css is the common part included by both
chrome_shared.css and chrome_shared2.css

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
File chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:1: // Copyright
(c) 2012 The Chromium Authors. All rights reserved.
On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> Please do a git cp
> chrome/browser/resources/options2/chromeos/accounts_options.js <this file> to
> minimize diff.

Done.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chromeos/ui_account_tweaks.cc (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.cc:7: #include "base/bind.h"
On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> Please cleanup headers. You don't need bind related headers, authenticator,
> policy etc.

Done.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.cc:27: * Fills given
dictionary with account status data (whether current user is
On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> This comment should be moved to header.

Done.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.cc:51: * Fills given data
source with account status data (whether current user is
On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> Remove / move to header.

Done.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chromeos/ui_account_tweaks.h (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:17: base::DictionaryValue*
localized_strings);
On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> nit: align

Done.

[Nikita (slow), 2012-04-09 12:18:49]

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/plugi...
File chrome/browser/resources/plugins.html (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/plugi...
chrome/browser/resources/plugins.html:6: <link rel="stylesheet"
href="chrome://resources/css/chrome_shared2.css">
On 2012/04/09 11:21:30, Denis Kuznetsov wrote:
> On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> > I don't see a new code in chrome_shared2.css for this patchset. Is this
change
> > for bringing chrome://plugins page style closer to other chrome:// pages
> style?
> chrome_shared2.css includes widgets.css that are altered by this CL.
> 

Right but at the same time it affects few styles on the page like checkbox and
that brings undesirable effect as broken layout.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
File chrome/browser/resources/shared/css/widgets.css (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
chrome/browser/resources/shared/css/widgets.css:220: <if
expr="pp_ifdef('chromeos')">
On 2012/04/09 11:21:30, Denis Kuznetsov wrote:
> On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> > chrome_shared2.css seems to be a better place for these rules.
> > 
> > As this file: "This file defines styles for form controls."
> > chrome_shared2.css: "This file holds CSS that should be shared, in theory,
by
> > all user-visible chrome:// pages. This contrasts chrome_shared.css, which it
> is
> > meant to replace, but has CSS specific to options and options-like pages."
> 
> On the other hand, widgets.css is the common part included by both
> chrome_shared.css and chrome_shared2.css

Good point. So it's either code duplication (placing it in chrome_shared.css and
chrome_shared2.css) or placing it in widgets.css.

Placing in widgets.css is fine for me but please add comment  on what these 2
styles are about.

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
File chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js (right):

http://codereview.chromium.org/10006028/diff/1/chrome/browser/resources/share...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:1: // Copyright
(c) 2012 The Chromium Authors. All rights reserved.
On 2012/04/09 11:21:30, Denis Kuznetsov wrote:
> On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> > Please do a git cp
> > chrome/browser/resources/options2/chromeos/accounts_options.js <this file>
to
> > minimize diff.
> 
> Done.

Ok, so it won't treat this as edit. Nm.

[Denis Kuznetsov (DE-MUC), 2012-04-09 13:17:03]

https://chromiumcodereview.appspot.com/10006028/diff/1/chrome/browser/resourc...
File chrome/browser/resources/plugins.html (right):

https://chromiumcodereview.appspot.com/10006028/diff/1/chrome/browser/resourc...
chrome/browser/resources/plugins.html:6: <link rel="stylesheet"
href="chrome://resources/css/chrome_shared2.css">
On 2012/04/09 12:18:49, Nikita Kostylev wrote:
> On 2012/04/09 11:21:30, Denis Kuznetsov wrote:
> > On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> > > I don't see a new code in chrome_shared2.css for this patchset. Is this
> change
> > > for bringing chrome://plugins page style closer to other chrome:// pages
> > style?
> > chrome_shared2.css includes widgets.css that are altered by this CL.
> > 
> 
> Right but at the same time it affects few styles on the page like checkbox and
> that brings undesirable effect as broken layout.

Done.

https://chromiumcodereview.appspot.com/10006028/diff/1/chrome/browser/resourc...
File chrome/browser/resources/shared/css/widgets.css (right):

https://chromiumcodereview.appspot.com/10006028/diff/1/chrome/browser/resourc...
chrome/browser/resources/shared/css/widgets.css:220: <if
expr="pp_ifdef('chromeos')">
On 2012/04/09 12:18:49, Nikita Kostylev wrote:
> On 2012/04/09 11:21:30, Denis Kuznetsov wrote:
> > On 2012/04/06 12:36:09, Nikita Kostylev wrote:
> > > chrome_shared2.css seems to be a better place for these rules.
> > > 
> > > As this file: "This file defines styles for form controls."
> > > chrome_shared2.css: "This file holds CSS that should be shared, in theory,
> by
> > > all user-visible chrome:// pages. This contrasts chrome_shared.css, which
it
> > is
> > > meant to replace, but has CSS specific to options and options-like pages."
> > 
> > On the other hand, widgets.css is the common part included by both
> > chrome_shared.css and chrome_shared2.css
> 
> Good point. So it's either code duplication (placing it in chrome_shared.css
and
> chrome_shared2.css) or placing it in widgets.css.
> 
> Placing in widgets.css is fine for me but please add comment  on what these 2
> styles are about.

Done.

[Nikita (slow), 2012-04-09 16:10:29]

lgtm but I think this might be actually done in a way that chrome_shared2.css is
still included at plugins.html page (we might want get there) but styles are
overridden properly.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared/css/chrome_shared2.css (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/css/chrome_shared2.css:10: <if
expr="pp_ifdef('chromeos')">
Why is this change needed?

[Denis Kuznetsov (DE-MUC), 2012-04-10 13:15:40]

James, friendly ping.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared/css/chrome_shared2.css (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/css/chrome_shared2.css:10: <if
expr="pp_ifdef('chromeos')">
On 2012/04/09 16:10:29, Nikita Kostylev wrote:
> Why is this change needed?
chrome://chrome/settings uber-page and some of it's subpages includes this CSS.
And this place looks like a nice single point of inclusion for new css.

[James Hawkins, 2012-04-10 18:26:54]

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/op...
File chrome/browser/resources/options2/options_bundle.js (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/op...
chrome/browser/resources/options2/options_bundle.js:16: <include
src="../shared/js/chromeos/ui_account_tweaks.js"></include>
Alphabetize.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/pl...
File chrome/browser/resources/plugins.html (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/pl...
chrome/browser/resources/plugins.html:8: <link rel="stylesheet"
href="chrome://resources/css/chromeos/ui_account_tweaks.css">
80 cols.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared/css/chrome_shared2.css (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/css/chrome_shared2.css:10: <if
expr="pp_ifdef('chromeos')">
On 2012/04/10 13:15:40, Denis Kuznetsov wrote:
> On 2012/04/09 16:10:29, Nikita Kostylev wrote:
> > Why is this change needed?
> chrome://chrome/settings uber-page and some of it's subpages includes this
CSS.
> And this place looks like a nice single point of inclusion for new css.

If it's only used in settings, it needs to go in the settings main CSS.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared/css/chromeos/ui_account_tweaks.css (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/css/chromeos/ui_account_tweaks.css:6: /* This
file defines styles for internal page elements that have special
This is only for settings, right?

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:27: /**
Indentation is off.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:35: * Returns
whether we're currently in guest mode.
@return

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:42: * Disable
or hide some elements in Guest mode in ChromeOS.
"Disables or hides"

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:56: if
(visibility == 'hidden') {
No braces for single-line blocks.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:105:
console.log(element);
Remove console spam.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared_resources.grd (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared_resources.grd:28:
file="shared/css/chrome_shared2.css" type="BINDATA" flattenhtml="true" />
80 cols.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared_resources.grd:168:
file="shared/css/chromeos/ui_account_tweaks.css" type="BINDATA" />
80 cols.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared_resources.grd:170:
file="shared/js/chromeos/ui_account_tweaks.js" type="BINDATA" />
80 cols.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/ui_account_tweaks.h (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:19: 
Remove blank line.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:21: base::DictionaryValue*
localized_strings);
Document |localized_strings|, notably that it must not be NULL.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:28: ChromeWebUIDataSource*
source);
This can't fit on the previous line?

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:28: ChromeWebUIDataSource*
source);
Document |source|.

[Denis Kuznetsov (DE-MUC), 2012-04-11 13:51:14]

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/op...
File chrome/browser/resources/options2/options_bundle.js (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/op...
chrome/browser/resources/options2/options_bundle.js:16: <include
src="../shared/js/chromeos/ui_account_tweaks.js"></include>
On 2012/04/10 18:26:54, James Hawkins wrote:
> Alphabetize.

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/pl...
File chrome/browser/resources/plugins.html (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/pl...
chrome/browser/resources/plugins.html:8: <link rel="stylesheet"
href="chrome://resources/css/chromeos/ui_account_tweaks.css">
On 2012/04/10 18:26:54, James Hawkins wrote:
> 80 cols.

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared/css/chrome_shared2.css (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/css/chrome_shared2.css:10: <if
expr="pp_ifdef('chromeos')">
On 2012/04/10 18:26:54, James Hawkins wrote:
> On 2012/04/10 13:15:40, Denis Kuznetsov wrote:
> > On 2012/04/09 16:10:29, Nikita Kostylev wrote:
> > > Why is this change needed?
> > chrome://chrome/settings uber-page and some of it's subpages includes this
> CSS.
> > And this place looks like a nice single point of inclusion for new css.
> 
> If it's only used in settings, it needs to go in the settings main CSS.
It is currently used by Options, and it will be used in Extensions (that also
includes chrome_shared2.css).

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared/css/chromeos/ui_account_tweaks.css (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/css/chromeos/ui_account_tweaks.css:6: /* This
file defines styles for internal page elements that have special
On 2012/04/10 18:26:54, James Hawkins wrote:
> This is only for settings, right?
It is used for Settings and Plugins, and there is an issue for disabling
elements on Extensions page.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:27: /**
On 2012/04/10 18:26:54, James Hawkins wrote:
> Indentation is off.

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:35: * Returns
whether we're currently in guest mode.
On 2012/04/10 18:26:54, James Hawkins wrote:
> @return

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:42: * Disable
or hide some elements in Guest mode in ChromeOS.
On 2012/04/10 18:26:54, James Hawkins wrote:
> "Disables or hides"

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:56: if
(visibility == 'hidden') {
On 2012/04/10 18:26:54, James Hawkins wrote:
> No braces for single-line blocks.

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared/js/chromeos/ui_account_tweaks.js:105:
console.log(element);
On 2012/04/10 18:26:54, James Hawkins wrote:
> Remove console spam.
Mea culpa.
Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
File chrome/browser/resources/shared_resources.grd (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared_resources.grd:28:
file="shared/css/chrome_shared2.css" type="BINDATA" flattenhtml="true" />
On 2012/04/10 18:26:54, James Hawkins wrote:
> 80 cols.

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared_resources.grd:168:
file="shared/css/chromeos/ui_account_tweaks.css" type="BINDATA" />
On 2012/04/10 18:26:54, James Hawkins wrote:
> 80 cols.

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/resources/sh...
chrome/browser/resources/shared_resources.grd:170:
file="shared/js/chromeos/ui_account_tweaks.js" type="BINDATA" />
On 2012/04/10 18:26:54, James Hawkins wrote:
> 80 cols.

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
File chrome/browser/ui/webui/chromeos/ui_account_tweaks.h (right):

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:19: 
On 2012/04/10 18:26:54, James Hawkins wrote:
> Remove blank line.

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:21: base::DictionaryValue*
localized_strings);
On 2012/04/10 18:26:54, James Hawkins wrote:
> Document |localized_strings|, notably that it must not be NULL.

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:28: ChromeWebUIDataSource*
source);
On 2012/04/10 18:26:54, James Hawkins wrote:
> This can't fit on the previous line?

Done.

http://codereview.chromium.org/10006028/diff/8005/chrome/browser/ui/webui/chr...
chrome/browser/ui/webui/chromeos/ui_account_tweaks.h:28: ChromeWebUIDataSource*
source);
On 2012/04/10 18:26:54, James Hawkins wrote:
> This can't fit on the previous line?

Done.

[James Hawkins, 2012-04-11 17:49:39]

I think you forgot to upload the next patch set.

[Denis Kuznetsov (DE-MUC), 2012-04-12 10:29:36]

On 2012/04/11 17:49:39, James Hawkins wrote:
> I think you forgot to upload the next patch set.

Sorry, I was sure I did.

[James Hawkins, 2012-04-13 03:39:23]

LGTM with nit.

http://codereview.chromium.org/10006028/diff/18001/chrome/browser/resources/p...
File chrome/browser/resources/plugins.html (right):

http://codereview.chromium.org/10006028/diff/18001/chrome/browser/resources/p...
chrome/browser/resources/plugins.html:9:
href="chrome://resources/css/chromeos/ui_account_tweaks.css">
nit: Indentation is off.  Should be 4 spaces from previous start column.

[commit-bot: I haz the power, 2012-04-13 11:04:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/antrim@chromium.org/10006028/23001

[commit-bot: I haz the power, 2012-04-13 12:40:42]

Change committed as 132177