Implement the spring charger replacement UI.

Implement the spring charger replacement UI and add related UMA stats.

This cl depends on
https://codereview.chromium.org/105923003/
to be land before it.

BUG=320708, 321419
R=derat@chromium.org, xiyuan@chromium.org
TBR=isherman, pam, sky

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=240486

[xiyuan, 2013-12-06 00:24:37]

Mostly nits.

https://codereview.chromium.org/107103004/diff/1/chrome/app/chromeos_strings....
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/107103004/diff/1/chrome/app/chromeos_strings....
chrome/app/chromeos_strings.grdp:3963: FAQs
This is not the right way to i18n the link. We should use <ph> here and use
i18n-values=".innerHTML:xxx" in html.

e.g.

In the meantime, you may refer to the <ph name="BEGIN_LINK">&lt;a
id="faq_link_id" href="#"&gt;</ph>FAQs<ph name="END_LINK">&lt;/a&gt;</ph> and
continue to use your HP Chromebook 11 with any other certified micro-USB ...

https://codereview.chromium.org/107103004/diff/1/chrome/browser/chromeos/char...
File chrome/browser/chromeos/charger_replace/charger_replacement_dialog.cc
(right):

https://codereview.chromium.org/107103004/diff/1/chrome/browser/chromeos/char...
chrome/browser/chromeos/charger_replace/charger_replacement_dialog.cc:29: const
char* kNewChargerOrdered = "1";
nit: use const char[] instead of *.

https://codereview.chromium.org/107103004/diff/1/chrome/browser/chromeos/char...
chrome/browser/chromeos/charger_replace/charger_replacement_dialog.cc:30: const
char* kNewChargerNotOrdered = "0";
nit: same here.

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/charger_replacement.css (right):

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/charger_replacement.css:6: padding: 8px;
nit: 2-space indent

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/charger_replacement.css:116: 
nit: nuke one empty line

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/charger_replacement.js (right):

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/charger_replacement.js:65: var
CHARGER_BOX_DESELECT_COLOR = '#999';
nit: move colors in css,

e.g.

.selected-charger {
  border-color: #4281f4;
}

and in JS later,

$('new-charger-box').classList.toggle('selected-charger', true/false); to use
the color.

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/charger_replacement.js:364: 
nit: nuke the empty line.

[jennyz, 2013-12-06 03:54:24]

https://codereview.chromium.org/107103004/diff/1/chrome/app/chromeos_strings....
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/107103004/diff/1/chrome/app/chromeos_strings....
chrome/app/chromeos_strings.grdp:3963: FAQs
On 2013/12/06 00:24:37, xiyuan wrote:
> This is not the right way to i18n the link. We should use <ph> here and use
> i18n-values=".innerHTML:xxx" in html.
> 
> e.g.
> 
> In the meantime, you may refer to the <ph name="BEGIN_LINK">&lt;a
> id="faq_link_id" href="#"&gt;</ph>FAQs<ph name="END_LINK">&lt;/a&gt;</ph> and
> continue to use your HP Chromebook 11 with any other certified micro-USB ...

Done.

https://codereview.chromium.org/107103004/diff/1/chrome/browser/chromeos/char...
File chrome/browser/chromeos/charger_replace/charger_replacement_dialog.cc
(right):

https://codereview.chromium.org/107103004/diff/1/chrome/browser/chromeos/char...
chrome/browser/chromeos/charger_replace/charger_replacement_dialog.cc:29: const
char* kNewChargerOrdered = "1";
On 2013/12/06 00:24:37, xiyuan wrote:
> nit: use const char[] instead of *.

Done.

https://codereview.chromium.org/107103004/diff/1/chrome/browser/chromeos/char...
chrome/browser/chromeos/charger_replace/charger_replacement_dialog.cc:30: const
char* kNewChargerNotOrdered = "0";
On 2013/12/06 00:24:37, xiyuan wrote:
> nit: same here.

Done.

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/charger_replacement.css (right):

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/charger_replacement.css:6: padding: 8px;
On 2013/12/06 00:24:37, xiyuan wrote:
> nit: 2-space indent

Done.

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/charger_replacement.css:116: 
On 2013/12/06 00:24:37, xiyuan wrote:
> nit: nuke one empty line

Done.

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/charger_replacement.js (right):

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/charger_replacement.js:65: var
CHARGER_BOX_DESELECT_COLOR = '#999';
On 2013/12/06 00:24:37, xiyuan wrote:
> nit: move colors in css,
> 
> e.g.
> 
> .selected-charger {
>   border-color: #4281f4;
> }
> 
> and in JS later,
> 
> $('new-charger-box').classList.toggle('selected-charger', true/false); to use
> the color.
> 

Done.

https://codereview.chromium.org/107103004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/charger_replacement.js:364: 
On 2013/12/06 00:24:37, xiyuan wrote:
> nit: nuke the empty line.

Done.

[jennyz, 2013-12-06 03:55:58]

derat@, please take a look at the change in:
ash/system/chromeos/power/. I tested with your powerd patch on spring device,
works fine.

[xiyuan, 2013-12-06 05:03:25]

LGTM with nits

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
File chrome/browser/resources/chromeos/charger_replacement.html (right):

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
chrome/browser/resources/chromeos/charger_replacement.html:138: <a class="link"
href="#">http://chromebook.com/hp11charger</a>
nit: Define the URL in C++ and set it here via i18n-contents

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
chrome/browser/resources/chromeos/charger_replacement.html:154: <a class="link"
href="#">http://chromebook.com/hp11charger</a>
same here.

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
chrome/browser/resources/chromeos/charger_replacement.html:211: <a class="link"
href="#">http://chromebook.com/hp11charger</a>
ditto

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
File chrome/browser/resources/chromeos/charger_replacement.js (right):

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
chrome/browser/resources/chromeos/charger_replacement.js:179:
charger.classList.toggle('de-selected-charger', false);
nit: we should be able to simplify this. We can make 'de-selected-charger' part
of the default style then we only need to toggle 'selected-charger' here. 

or at least, we can get rid of the "if" and just do
  charger.classList.toggle('selected-charger', selected);
  charger.classList.toggle('de-selected-charger', !selected);

[Daniel Erat, 2013-12-06 17:43:25]

just looked at ash/system/chromeos

https://codereview.chromium.org/107103004/diff/60001/ash/system/chromeos/powe...
File ash/system/chromeos/power/power_status.cc (right):

https://codereview.chromium.org/107103004/diff/60001/ash/system/chromeos/powe...
ash/system/chromeos/power/power_status.cc:194: // in purpose of backport
compatibility.
nit: add a TODO to change this to
PowerSupplyProperties_ExternalPower_ORIGINAL_SPRING_CHARGER on trunk after the
change has been merged?

https://codereview.chromium.org/107103004/diff/60001/ash/system/chromeos/powe...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/107103004/diff/60001/ash/system/chromeos/powe...
ash/system/chromeos/power/tray_power.cc:307: void
TrayPower::ShowChargerReplacementDialogIfNecessary() {
this separate method seems unnecessary; mind just moving this to
OnPowerStatusChanged()?

  if (PowerStatus::Get()->IsOriginalSpringChargerConnected()) {
    ash::Shell::GetInstance()->system_tray_delegate()->
        ShowSpringChargerReplacementDialog();
  }

[jennyz, 2013-12-06 18:15:15]

https://codereview.chromium.org/107103004/diff/60001/ash/system/chromeos/powe...
File ash/system/chromeos/power/power_status.cc (right):

https://codereview.chromium.org/107103004/diff/60001/ash/system/chromeos/powe...
ash/system/chromeos/power/power_status.cc:194: // in purpose of backport
compatibility.
On 2013/12/06 17:43:26, Daniel Erat wrote:
> nit: add a TODO to change this to
> PowerSupplyProperties_ExternalPower_ORIGINAL_SPRING_CHARGER on trunk after the
> change has been merged?

Done.

https://codereview.chromium.org/107103004/diff/60001/ash/system/chromeos/powe...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/107103004/diff/60001/ash/system/chromeos/powe...
ash/system/chromeos/power/tray_power.cc:307: void
TrayPower::ShowChargerReplacementDialogIfNecessary() {
On 2013/12/06 17:43:26, Daniel Erat wrote:
> this separate method seems unnecessary; mind just moving this to
> OnPowerStatusChanged()?
> 
>   if (PowerStatus::Get()->IsOriginalSpringChargerConnected()) {
>     ash::Shell::GetInstance()->system_tray_delegate()->
>         ShowSpringChargerReplacementDialog();
>   }

Done.

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
File chrome/browser/resources/chromeos/charger_replacement.html (right):

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
chrome/browser/resources/chromeos/charger_replacement.html:138: <a class="link"
href="#">http://chromebook.com/hp11charger</a>
On 2013/12/06 05:03:26, xiyuan wrote:
> nit: Define the URL in C++ and set it here via i18n-contents

Done.

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
chrome/browser/resources/chromeos/charger_replacement.html:154: <a class="link"
href="#">http://chromebook.com/hp11charger</a>
On 2013/12/06 05:03:26, xiyuan wrote:
> same here.

Done.

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
chrome/browser/resources/chromeos/charger_replacement.html:211: <a class="link"
href="#">http://chromebook.com/hp11charger</a>
On 2013/12/06 05:03:26, xiyuan wrote:
> ditto

Done.

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
File chrome/browser/resources/chromeos/charger_replacement.js (right):

https://codereview.chromium.org/107103004/diff/60001/chrome/browser/resources...
chrome/browser/resources/chromeos/charger_replacement.js:179:
charger.classList.toggle('de-selected-charger', false);
On 2013/12/06 05:03:26, xiyuan wrote:
> nit: we should be able to simplify this. We can make 'de-selected-charger'
part
> of the default style then we only need to toggle 'selected-charger' here. 
> 
> or at least, we can get rid of the "if" and just do
>   charger.classList.toggle('selected-charger', selected);
>   charger.classList.toggle('de-selected-charger', !selected);

I was making de-selected-charger default style, but then toggling on the
selected-charger class would not overwrite the border color. Anyway, I
simplified this function.

[Daniel Erat, 2013-12-06 18:45:24]

LGTM for ash/system/chromeos/power

https://codereview.chromium.org/107103004/diff/80001/ash/system/chromeos/powe...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/107103004/diff/80001/ash/system/chromeos/powe...
ash/system/chromeos/power/tray_power.cc:167: // Show spring charger replacement
dialog if necessary.
nit: remove unnecessary comment

[jennyz, 2013-12-06 19:11:35]

https://codereview.chromium.org/107103004/diff/80001/ash/system/chromeos/powe...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/107103004/diff/80001/ash/system/chromeos/powe...
ash/system/chromeos/power/tray_power.cc:167: // Show spring charger replacement
dialog if necessary.
On 2013/12/06 18:45:24, Daniel Erat wrote:
> nit: remove unnecessary comment

Done.

[jennyz, 2013-12-09 23:48:42]

PTAL.

[Daniel Erat, 2013-12-10 00:08:58]

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:171: if
(PowerStatus::Get()->IsOriginalSpringChargerConnected()) {
shouldn't this also check !HasUserConfirmedSafeSpringCharger()?

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:173:
ShowSpringChargerReplacementDialog();
should the dialog be hidden when the charger is unplugged?

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:336: current_charger = UNKNOWN_CHARGER;
nit: if you just initialize current_charger to UNKNOWN_CHARGER above, you can
remove this else clause

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:340:
UMA_HISTOGRAM_ENUMERATION("PowerSupply.ExternalPowerSupply",
where did this name come from? all of the histograms in powerd have names like
"Power.Whatever"; could you name this something like "Power.ChargerType"
instead?

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tray_power.h (right):

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:65: ORIGINAL_SPRING_CHARGER,
i think that the terminology here is getting confusing. i believe that you're
using "original" to describe chargers that have the original id but where the
presence of the sticker is unknown, while "safe" refers to chargers with the
original id and with a sticker. spring chargers with updated ids will use
MAINS_CHARGER instead.

at the very least, you should add comments to the enum explaining this, but i'd
suggest more-descriptive names as well. how about something like this?

UNCONFIRMED_SPRING_CHARGER
SAFE_SPRING_CHARGER

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:95: void RecordExternalPowerSupply();
nit: rename to something like RecordChargerType()?

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:106: bool is_line_power_connected_;
add a comment describing what this is and match the naming style of the member
right above it, i.e. line_power_was_connected_

but it might be better to just name it something like
already_recorded_charger_type_

[xiyuan, 2013-12-10 00:21:19]

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tray_power.h (right):

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:67: NUM_EXTERNAL_POWER_SUPPLY,
From what I learned from isherman@ on another CL, it would be better to use an
addition bucket here in case we need to add more bucket in the future. That is,
have a bucket id NUM_EXTERNAL_POWER_COUNT (which is NUM_EXTERNAL_POWER_SUPPLY +
1) here.

[jennyz, 2013-12-10 19:14:11]

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:171: if
(PowerStatus::Get()->IsOriginalSpringChargerConnected()) {
On 2013/12/10 00:08:58, Daniel Erat wrote:
> shouldn't this also check !HasUserConfirmedSafeSpringCharger()?

The logic for checking if the dialog should be shown is in
ChargerReplacementDialog::ShouldShowDialog, called by
SystemTrayDelegate::ShowSpringChargerReplacementDialog. We won't show the dialog
unless user confirms they are using safe charger, or they decline to order a
replacement charger, or they ordered the charger but confirm that they would
like keep using the original charger. The flow gets more complicated later than
we originally discussed.

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:173:
ShowSpringChargerReplacementDialog();
On 2013/12/10 00:08:58, Daniel Erat wrote:
> should the dialog be hidden when the charger is unplugged?

Once the dialog is shown, user has to complete the flow. We won't hide it when
the charger is unplugged, that's kind of confusing.

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:336: current_charger = UNKNOWN_CHARGER;
On 2013/12/10 00:08:58, Daniel Erat wrote:
> nit: if you just initialize current_charger to UNKNOWN_CHARGER above, you can
> remove this else clause

Done.

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:340:
UMA_HISTOGRAM_ENUMERATION("PowerSupply.ExternalPowerSupply",
On 2013/12/10 00:08:58, Daniel Erat wrote:
> where did this name come from? all of the histograms in powerd have names like
> "Power.Whatever"; could you name this something like "Power.ChargerType"
> instead?

Done.

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tray_power.h (right):

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:65: ORIGINAL_SPRING_CHARGER,
On 2013/12/10 00:08:58, Daniel Erat wrote:
> i think that the terminology here is getting confusing. i believe that you're
> using "original" to describe chargers that have the original id but where the
> presence of the sticker is unknown, while "safe" refers to chargers with the
> original id and with a sticker. spring chargers with updated ids will use
> MAINS_CHARGER instead.
> 
> at the very least, you should add comments to the enum explaining this, but
i'd
> suggest more-descriptive names as well. how about something like this?
> 
> UNCONFIRMED_SPRING_CHARGER
> SAFE_SPRING_CHARGER

Done.

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:67: NUM_EXTERNAL_POWER_SUPPLY,
On 2013/12/10 00:21:19, xiyuan wrote:
> From what I learned from isherman@ on another CL, it would be better to use an
> addition bucket here in case we need to add more bucket in the future. That
is,
> have a bucket id NUM_EXTERNAL_POWER_COUNT (which is NUM_EXTERNAL_POWER_SUPPLY
+
> 1) here.
The last enum value is already the count, renamed it to CHARGER_TYPE_COUNT.

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:95: void RecordExternalPowerSupply();
On 2013/12/10 00:08:58, Daniel Erat wrote:
> nit: rename to something like RecordChargerType()?

Done.

https://codereview.chromium.org/107103004/diff/120001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:106: bool is_line_power_connected_;
On 2013/12/10 00:08:58, Daniel Erat wrote:
> add a comment describing what this is and match the naming style of the member
> right above it, i.e. line_power_was_connected_
> 
> but it might be better to just name it something like
> already_recorded_charger_type_

Done.

[Daniel Erat, 2013-12-10 19:21:54]

https://codereview.chromium.org/107103004/diff/160001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/107103004/diff/160001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:319: }
you can remove this if you follow the below suggestion

https://codereview.chromium.org/107103004/diff/160001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:321: if (line_power_was_connected_)
you can change this to:

  if (!PowerStatus::Get()->IsLinePowerConnected() ||
      line_power_was_connected_)
    return;

if you follow the below suggestion

https://codereview.chromium.org/107103004/diff/160001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:324: line_power_was_connected_ = true;
move this line to the end of OnPowerStatusChanged(), where
usb_charger_was_connected_ is updated

[jennyz, 2013-12-10 19:50:13]

https://codereview.chromium.org/107103004/diff/160001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tray_power.cc (right):

https://codereview.chromium.org/107103004/diff/160001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:319: }
On 2013/12/10 19:21:54, Daniel Erat wrote:
> you can remove this if you follow the below suggestion

Done.

https://codereview.chromium.org/107103004/diff/160001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:321: if (line_power_was_connected_)
On 2013/12/10 19:21:54, Daniel Erat wrote:
> you can change this to:
> 
>   if (!PowerStatus::Get()->IsLinePowerConnected() ||
>       line_power_was_connected_)
>     return;
> 
> if you follow the below suggestion

Done.

https://codereview.chromium.org/107103004/diff/160001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.cc:324: line_power_was_connected_ = true;
On 2013/12/10 19:21:54, Daniel Erat wrote:
> move this line to the end of OnPowerStatusChanged(), where
> usb_charger_was_connected_ is updated

Done.

[Daniel Erat, 2013-12-10 21:30:35]

LGTM for ash/system/chromeos/power

https://codereview.chromium.org/107103004/diff/180001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tray_power.h (right):

https://codereview.chromium.org/107103004/diff/180001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:61: enum ChargerType{
nit: add a comment explaining that existing values shouldn't be removed and that
new values should be added just before CHARGER_TYPE_COUNT because this enum is
used for a histogram

https://codereview.chromium.org/107103004/diff/180001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/107103004/diff/180001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:13771: +    UNCONFIRMED_SPRING_CHARGER,
SAFE_SPRING_CHARGER.
nit: document that a sample is reported every time a charger is connected to the
device

[jennyz, 2013-12-10 22:05:20]

https://codereview.chromium.org/107103004/diff/180001/ash/system/chromeos/pow...
File ash/system/chromeos/power/tray_power.h (right):

https://codereview.chromium.org/107103004/diff/180001/ash/system/chromeos/pow...
ash/system/chromeos/power/tray_power.h:61: enum ChargerType{
On 2013/12/10 21:30:36, Daniel Erat wrote:
> nit: add a comment explaining that existing values shouldn't be removed and
that
> new values should be added just before CHARGER_TYPE_COUNT because this enum is
> used for a histogram

Done.

[xiyuan, 2013-12-10 22:22:23]

SLGTM

[commit-bot: I haz the power, 2013-12-12 18:38:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennyz@chromium.org/107103004/200001

[commit-bot: I haz the power, 2013-12-12 18:39:00]

Failed to apply patch for
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: chrome/browser/chromeos/system/ash_system_tray_delegate.cc
  |diff --git a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
  |index
1e3fcf00a237a9837f59ca2f628906f0386fe648..59c7ca4e1d2cb13ef1a82bcf605084d24953b38c
100644
  |--- a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
  |+++ b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
  --------------------------
  No file to patch.  Skipping patch.
  3 out of 3 hunks ignored

Patch:       chrome/browser/chromeos/system/ash_system_tray_delegate.cc
Index: chrome/browser/chromeos/system/ash_system_tray_delegate.cc
diff --git a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
index
1e3fcf00a237a9837f59ca2f628906f0386fe648..59c7ca4e1d2cb13ef1a82bcf605084d24953b38c
100644
--- a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
+++ b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
@@ -46,6 +46,7 @@
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/accessibility/magnification_manager.h"
 #include "chrome/browser/chromeos/bluetooth/bluetooth_pairing_dialog.h"
+#include "chrome/browser/chromeos/charger_replace/charger_replacement_dialog.h"
 #include "chrome/browser/chromeos/choose_mobile_network_dialog.h"
 #include "chrome/browser/chromeos/drive/drive_integration_service.h"
 #include "chrome/browser/chromeos/drive/job_list.h"
@@ -80,6 +81,7 @@
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/webui/chromeos/charger_replacement_handler.h"
 #include "chrome/browser/ui/webui/chromeos/mobile_setup_dialog.h"
 #include "chrome/browser/upgrade_detector.h"
 #include "chrome/common/pref_names.h"
@@ -617,6 +619,20 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
       UserAddingScreen::Get()->Start();
   }
 
+  virtual void ShowSpringChargerReplacementDialog() OVERRIDE {
+    if (!ChargerReplacementDialog::ShouldShowDialog())
+      return;
+
+    ChargerReplacementDialog* dialog =
+        new ChargerReplacementDialog(GetNativeWindow());
+    dialog->Show();
+  }
+
+  virtual bool HasUserConfirmedSafeSpringCharger() OVERRIDE {
+    return ChargerReplacementHandler::GetChargerStatusPref() ==
+        ChargerReplacementHandler::CONFIRM_SAFE_CHARGER;
+  }
+
   virtual void ShutDown() OVERRIDE {
     ash::Shell::GetInstance()->lock_state_controller()->RequestShutdown();
   }

[commit-bot: I haz the power, 2013-12-12 18:46:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennyz@chromium.org/107103004/200001

[commit-bot: I haz the power, 2013-12-12 18:46:50]

Failed to apply patch for
chrome/browser/chromeos/system/ash_system_tray_delegate.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index: chrome/browser/chromeos/system/ash_system_tray_delegate.cc
  |diff --git a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
  |index
1e3fcf00a237a9837f59ca2f628906f0386fe648..59c7ca4e1d2cb13ef1a82bcf605084d24953b38c
100644
  |--- a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
  |+++ b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
  --------------------------
  No file to patch.  Skipping patch.
  3 out of 3 hunks ignored

Patch:       chrome/browser/chromeos/system/ash_system_tray_delegate.cc
Index: chrome/browser/chromeos/system/ash_system_tray_delegate.cc
diff --git a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
index
1e3fcf00a237a9837f59ca2f628906f0386fe648..59c7ca4e1d2cb13ef1a82bcf605084d24953b38c
100644
--- a/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
+++ b/chrome/browser/chromeos/system/ash_system_tray_delegate.cc
@@ -46,6 +46,7 @@
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/accessibility/magnification_manager.h"
 #include "chrome/browser/chromeos/bluetooth/bluetooth_pairing_dialog.h"
+#include "chrome/browser/chromeos/charger_replace/charger_replacement_dialog.h"
 #include "chrome/browser/chromeos/choose_mobile_network_dialog.h"
 #include "chrome/browser/chromeos/drive/drive_integration_service.h"
 #include "chrome/browser/chromeos/drive/job_list.h"
@@ -80,6 +81,7 @@
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/webui/chromeos/charger_replacement_handler.h"
 #include "chrome/browser/ui/webui/chromeos/mobile_setup_dialog.h"
 #include "chrome/browser/upgrade_detector.h"
 #include "chrome/common/pref_names.h"
@@ -617,6 +619,20 @@ class SystemTrayDelegate : public ash::SystemTrayDelegate,
       UserAddingScreen::Get()->Start();
   }
 
+  virtual void ShowSpringChargerReplacementDialog() OVERRIDE {
+    if (!ChargerReplacementDialog::ShouldShowDialog())
+      return;
+
+    ChargerReplacementDialog* dialog =
+        new ChargerReplacementDialog(GetNativeWindow());
+    dialog->Show();
+  }
+
+  virtual bool HasUserConfirmedSafeSpringCharger() OVERRIDE {
+    return ChargerReplacementHandler::GetChargerStatusPref() ==
+        ChargerReplacementHandler::CONFIRM_SAFE_CHARGER;
+  }
+
   virtual void ShutDown() OVERRIDE {
     ash::Shell::GetInstance()->lock_state_controller()->RequestShutdown();
   }

[commit-bot: I haz the power, 2013-12-12 19:35:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennyz@chromium.org/107103004/200001

[commit-bot: I haz the power, 2013-12-12 19:54:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennyz@chromium.org/107103004/210001

[commit-bot: I haz the power, 2013-12-12 21:01:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennyz@chromium.org/107103004/230001

[commit-bot: I haz the power, 2013-12-12 23:41:30]

Commit queue failed due to new patchset.

[commit-bot: I haz the power, 2013-12-12 23:45:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennyz@chromium.org/107103004/250001

[jennyz, 2013-12-12 23:56:03]

Committed patchset #11 manually as r240486 (presubmit successful).