Add update available icon in system tray

Add update available icon in system tray

- Show an update available icon in system tray upon receiving
NEED_PERMISSION_TO_UPDATE state from update engine.

- This icon brings user to the about Chrome OS page and prompts update
warning window with update size.

- Design doc: go/cros-cellular-updates

BUG=chromium:691108
TEST=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2882933002
Cr-Commit-Position: refs/heads/master@{#473614}
Committed: https://chromium.googlesource.com/chromium/src/+/ef06a6f4d0434cbc277103b953ed33b12d0d0261

[weidongg, 2017-05-15 04:54:12]

weidongg@chromium.org changed reviewers:
+ stevenjb@chromium.org

[weidongg, 2017-05-15 04:54:13]

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
File ash/system/update/tray_update.cc (right):

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
ash/system/update/tray_update.cc:124:
Shell::Get()->system_tray_controller()->ShowAboutChromeOS();
Here I want to show the about Chrome OS page first. After the page is loaded
completely, the RequestUpdateCheck() is invoked, but I am not sure how to do
this.

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
File ash/system/update/tray_update.h (right):

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
ash/system/update/tray_update.h:64: static bool upgrade_required_;
I refactored here to distinguish 'update' with 'upgrade'. It seems upgrade does
not include the downloading process of a software update, but I am not sure.

[stevenjb, 2017-05-15 18:06:45]

https://codereview.chromium.org/2882933002/diff/1/ash/public/interfaces/syste...
File ash/public/interfaces/system_tray.mojom (right):

https://codereview.chromium.org/2882933002/diff/1/ash/public/interfaces/syste...
ash/public/interfaces/system_tray.mojom:66: ShowAboutChromeOS();
nit: Put this next to 'ShowHelp', and update the comments as follows. 

// Show the 'About Chrome OS' page in the Settings UI.
ShowAboutChromeOS();

// Show the Chromebook help app.
ShowHelp();

(The difference between 'help' and 'about' is often confused, e.g. even though
the Settings page is 'About Chrome OS', the URL for it is (for historical
reasons) chrome://settings/help. That's why I want to clarify these here.)

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
File ash/system/update/tray_update.cc (right):

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
ash/system/update/tray_update.cc:124:
Shell::Get()->system_tray_controller()->ShowAboutChromeOS();
On 2017/05/15 04:54:13, weidongg wrote:
> Here I want to show the about Chrome OS page first. After the page is loaded
> completely, the RequestUpdateCheck() is invoked, but I am not sure how to do
> this.

What is the reason for wanting to show the about page first? Triggering both at
the same time seems OK to me...

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
ash/system/update/tray_update.cc:161: upgrade_required_ = true;
Switching to 'upgrade' just seems confusing to me. We don't use that term here
(or anywhere in this code that I am aware if). |update_required_| seems the most
clear to me.

https://codereview.chromium.org/2882933002/diff/1/chrome/common/url_constants.h
File chrome/common/url_constants.h (right):

https://codereview.chromium.org/2882933002/diff/1/chrome/common/url_constants...
chrome/common/url_constants.h:358: extern const char kHelpSubPage[];
I guess this is fine for now but FYI I just filed crbug.com/722450 to change
this to chrome://settings/about to better match the title and usage of the page.

[weidongg, 2017-05-15 20:06:58]

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
File ash/system/update/tray_update.cc (right):

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
ash/system/update/tray_update.cc:124:
Shell::Get()->system_tray_controller()->ShowAboutChromeOS();
On 2017/05/15 18:06:45, stevenjb wrote:
> On 2017/05/15 04:54:13, weidongg wrote:
> > Here I want to show the about Chrome OS page first. After the page is loaded
> > completely, the RequestUpdateCheck() is invoked, but I am not sure how to do
> > this.
> 
> What is the reason for wanting to show the about page first? Triggering both
at
> the same time seems OK to me...

In the design doc, clicking this icon will bring user to About Chrome OS page
and show an update warning dialog above the page.
But that dialog is only triggered when the page receives signal from update
engine. The signal is broadcasted after RequestUpdateCheck() is called.
So the sequence should be:
About Chrome OS page is loaded completely -> RequestUpdateCheck() is called ->
NEED_PERMISSION_TO_UPDATE is broadcasted by update engine -> About Chrome OS
page shows update warning dialog.

https://codereview.chromium.org/2882933002/diff/1/ash/system/update/tray_upda...
ash/system/update/tray_update.cc:161: upgrade_required_ = true;
On 2017/05/15 18:06:45, stevenjb wrote:
> Switching to 'upgrade' just seems confusing to me. We don't use that term here
> (or anywhere in this code that I am aware if). |update_required_| seems the
most
> clear to me.

Ok, I see, I will refactor it back.

https://codereview.chromium.org/2882933002/diff/1/chrome/common/url_constants.h
File chrome/common/url_constants.h (right):

https://codereview.chromium.org/2882933002/diff/1/chrome/common/url_constants...
chrome/common/url_constants.h:358: extern const char kHelpSubPage[];
On 2017/05/15 18:06:45, stevenjb wrote:
> I guess this is fine for now but FYI I just filed crbug.com/722450 to change
> this to chrome://settings/about to better match the title and usage of the
page.

Ok, I see. Should I rename it to kAboutChromeOSPage?

[stevenjb, 2017-05-15 20:25:28]

https://codereview.chromium.org/2882933002/diff/1/chrome/common/url_constants.h
File chrome/common/url_constants.h (right):

https://codereview.chromium.org/2882933002/diff/1/chrome/common/url_constants...
chrome/common/url_constants.h:358: extern const char kHelpSubPage[];
On 2017/05/15 20:06:57, weidongg wrote:
> On 2017/05/15 18:06:45, stevenjb wrote:
> > I guess this is fine for now but FYI I just filed crbug.com/722450 to change
> > this to chrome://settings/about to better match the title and usage of the
> page.
> 
> Ok, I see. Should I rename it to kAboutChromeOSPage?

We could, but I think that might be confusing, so maybe wait until we change the
url? Whichever seems less confusing to you.

[weidongg, 2017-05-16 19:46:08]

https://codereview.chromium.org/2882933002/diff/1/ash/public/interfaces/syste...
File ash/public/interfaces/system_tray.mojom (right):

https://codereview.chromium.org/2882933002/diff/1/ash/public/interfaces/syste...
ash/public/interfaces/system_tray.mojom:66: ShowAboutChromeOS();
On 2017/05/15 18:06:45, stevenjb wrote:
> nit: Put this next to 'ShowHelp', and update the comments as follows. 
> 
> // Show the 'About Chrome OS' page in the Settings UI.
> ShowAboutChromeOS();
> 
> // Show the Chromebook help app.
> ShowHelp();
> 
> (The difference between 'help' and 'about' is often confused, e.g. even though
> the Settings page is 'About Chrome OS', the URL for it is (for historical
> reasons) chrome://settings/help. That's why I want to clarify these here.)
> 

Done.

https://codereview.chromium.org/2882933002/diff/1/chrome/common/url_constants.h
File chrome/common/url_constants.h (right):

https://codereview.chromium.org/2882933002/diff/1/chrome/common/url_constants...
chrome/common/url_constants.h:358: extern const char kHelpSubPage[];
On 2017/05/15 20:25:28, stevenjb wrote:
> On 2017/05/15 20:06:57, weidongg wrote:
> > On 2017/05/15 18:06:45, stevenjb wrote:
> > > I guess this is fine for now but FYI I just filed crbug.com/722450 to
change
> > > this to chrome://settings/about to better match the title and usage of the
> > page.
> > 
> > Ok, I see. Should I rename it to kAboutChromeOSPage?
> 
> We could, but I think that might be confusing, so maybe wait until we change
the
> url? Whichever seems less confusing to you.

Ok, I see.

[stevenjb, 2017-05-16 19:49:07]

https://codereview.chromium.org/2882933002/diff/20001/ash/public/interfaces/s...
File ash/public/interfaces/system_tray.mojom (right):

https://codereview.chromium.org/2882933002/diff/20001/ash/public/interfaces/s...
ash/public/interfaces/system_tray.mojom:80: // Show the about chrome OS page.
Shows

https://codereview.chromium.org/2882933002/diff/20001/ash/public/interfaces/s...
ash/public/interfaces/system_tray.mojom:83: // Shows help.
Change this to: // Shows the Chromebook help app.

https://codereview.chromium.org/2882933002/diff/20001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_client.cc (right):

https://codereview.chromium.org/2882933002/diff/20001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_client.cc:194: // page is loaded.
What are we delaying this? Please explain. (Also combine comment lines)

[weidongg, 2017-05-16 19:58:37]

https://codereview.chromium.org/2882933002/diff/20001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_client.cc (right):

https://codereview.chromium.org/2882933002/diff/20001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_client.cc:194: // page is loaded.
On 2017/05/16 19:49:06, stevenjb wrote:
> What are we delaying this? Please explain. (Also combine comment lines)

Clicking this icon will bring user to About Chrome OS page
and show an update warning dialog above the page.
But that dialog is only triggered when the page receives signal from update
engine. The signal is broadcasted after RequestUpdateCheck() is called.
So the sequence should be:
About Chrome OS page is loaded completely -> RequestUpdateCheck() is called ->
NEED_PERMISSION_TO_UPDATE is broadcasted by update engine -> About Chrome OS
page shows update warning dialog.

There's no callback when the page is completely loaded, so my solution is to
delay for a certain of time, but I am not sure whether 1 second is proper.

[stevenjb, 2017-05-16 20:42:34]

https://codereview.chromium.org/2882933002/diff/20001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_client.cc (right):

https://codereview.chromium.org/2882933002/diff/20001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_client.cc:194: // page is loaded.
On 2017/05/16 19:58:36, weidongg wrote:
> On 2017/05/16 19:49:06, stevenjb wrote:
> > What are we delaying this? Please explain. (Also combine comment lines)
> 
> Clicking this icon will bring user to About Chrome OS page
> and show an update warning dialog above the page.
> But that dialog is only triggered when the page receives signal from update
> engine. The signal is broadcasted after RequestUpdateCheck() is called.
> So the sequence should be:
> About Chrome OS page is loaded completely -> RequestUpdateCheck() is called ->
> NEED_PERMISSION_TO_UPDATE is broadcasted by update engine -> About Chrome OS
> page shows update warning dialog.
> 
> There's no callback when the page is completely loaded, so my solution is to
> delay for a certain of time, but I am not sure whether 1 second is proper.

Using a time delay like this is fragile at best.

We should modify the page to take a parameter, e.g. request_update=true and have
the page request the update instead.

See internet_detail_page.js for an example where we use getQueryParameters() to
show show a configuration dialog.

[weidongg, 2017-05-17 00:25:53]

Description was changed from

==========
Add update available icon in system tray

- Show an update available icon in system tray upon receiving
NEED_PERMISSION_TO_UPDATE state from update engine.

- This icon brings user to the about Chrome OS page and prompts update
warning window with update size.

- Design doc: go/cros-cellular-updates

BUG=chromium:691108
TEST=None
==========

to

==========
Add update available icon in system tray

- Show an update available icon in system tray upon receiving
NEED_PERMISSION_TO_UPDATE state from update engine.

- This icon brings user to the about Chrome OS page and prompts update
warning window with update size.

- Design doc: go/cros-cellular-updates

BUG=chromium:691108
TEST=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[weidongg, 2017-05-17 00:27:23]

https://codereview.chromium.org/2882933002/diff/20001/ash/public/interfaces/s...
File ash/public/interfaces/system_tray.mojom (right):

https://codereview.chromium.org/2882933002/diff/20001/ash/public/interfaces/s...
ash/public/interfaces/system_tray.mojom:80: // Show the about chrome OS page.
On 2017/05/16 19:49:06, stevenjb wrote:
> Shows

Done.

https://codereview.chromium.org/2882933002/diff/20001/ash/public/interfaces/s...
ash/public/interfaces/system_tray.mojom:83: // Shows help.
On 2017/05/16 19:49:06, stevenjb wrote:
> Change this to: // Shows the Chromebook help app.

Done.

https://codereview.chromium.org/2882933002/diff/20001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_client.cc (right):

https://codereview.chromium.org/2882933002/diff/20001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_client.cc:194: // page is loaded.
On 2017/05/16 20:42:34, stevenjb wrote:
> On 2017/05/16 19:58:36, weidongg wrote:
> > On 2017/05/16 19:49:06, stevenjb wrote:
> > > What are we delaying this? Please explain. (Also combine comment lines)
> > 
> > Clicking this icon will bring user to About Chrome OS page
> > and show an update warning dialog above the page.
> > But that dialog is only triggered when the page receives signal from update
> > engine. The signal is broadcasted after RequestUpdateCheck() is called.
> > So the sequence should be:
> > About Chrome OS page is loaded completely -> RequestUpdateCheck() is called
->
> > NEED_PERMISSION_TO_UPDATE is broadcasted by update engine -> About Chrome OS
> > page shows update warning dialog.
> > 
> > There's no callback when the page is completely loaded, so my solution is to
> > delay for a certain of time, but I am not sure whether 1 second is proper.
> 
> Using a time delay like this is fragile at best.
> 
> We should modify the page to take a parameter, e.g. request_update=true and
have
> the page request the update instead.
> 
> See internet_detail_page.js for an example where we use getQueryParameters()
to
> show show a configuration dialog.

Yes, I was worried about the fragility too. Making the page take a parameter is
much better. Thanks.

[stevenjb, 2017-05-17 16:56:32]

Better, thanks!

https://codereview.chromium.org/2882933002/diff/40001/ash/system/update/tray_...
File ash/system/update/tray_update.cc (right):

https://codereview.chromium.org/2882933002/diff/40001/ash/system/update/tray_...
ash/system/update/tray_update.cc:124: true /* check_for_update */);
This appears to be the only call to ShowAboutChromeOS? If so, remove the
parameter (and update the comment and optionally rename it
ShowAboutChromeOSAndCheckForUpdates, but that's kind of long, a comment is
fine).

https://codereview.chromium.org/2882933002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/about_page/about_page.js (right):

https://codereview.chromium.org/2882933002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/about_page/about_page.js:155: if
(checkForUpdate == 'true') {
if (settings.getQueryParameters().get('checkForUpdate') == 'true')

https://codereview.chromium.org/2882933002/diff/40001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_client.cc (right):

https://codereview.chromium.org/2882933002/diff/40001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_client.cc:303: sub_page += (check_for_update ?
"?checkForUpdate=true" : "");
With the parameter removed, you can just add a comment:

// We always want to check for updates when showing the about page from the Ash
UI
ShowSettingsSubPageForActiveUser(chrome::kHelpSubPage + "?checkForUpdate=true");

[weidongg, 2017-05-17 18:33:37]

https://codereview.chromium.org/2882933002/diff/40001/ash/system/update/tray_...
File ash/system/update/tray_update.cc (right):

https://codereview.chromium.org/2882933002/diff/40001/ash/system/update/tray_...
ash/system/update/tray_update.cc:124: true /* check_for_update */);
On 2017/05/17 16:56:31, stevenjb wrote:
> This appears to be the only call to ShowAboutChromeOS? If so, remove the
> parameter (and update the comment and optionally rename it
> ShowAboutChromeOSAndCheckForUpdates, but that's kind of long, a comment is
> fine).

Yes, this is the only call. Done.

https://codereview.chromium.org/2882933002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/about_page/about_page.js (right):

https://codereview.chromium.org/2882933002/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/about_page/about_page.js:155: if
(checkForUpdate == 'true') {
On 2017/05/17 16:56:31, stevenjb wrote:
> if (settings.getQueryParameters().get('checkForUpdate') == 'true')

Done.

https://codereview.chromium.org/2882933002/diff/40001/chrome/browser/ui/ash/s...
File chrome/browser/ui/ash/system_tray_client.cc (right):

https://codereview.chromium.org/2882933002/diff/40001/chrome/browser/ui/ash/s...
chrome/browser/ui/ash/system_tray_client.cc:303: sub_page += (check_for_update ?
"?checkForUpdate=true" : "");
On 2017/05/17 16:56:31, stevenjb wrote:
> With the parameter removed, you can just add a comment:
> 
> // We always want to check for updates when showing the about page from the
Ash
> UI
> ShowSettingsSubPageForActiveUser(chrome::kHelpSubPage +
"?checkForUpdate=true");

Done.

[stevenjb, 2017-05-17 19:11:37]

lgtm

[weidongg, 2017-05-17 19:15:32]

The CQ bit was checked by weidongg@chromium.org

[commit-bot: I haz the power, 2017-05-17 19:17:02]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-17 19:53:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-17 19:53:18]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[weidongg, 2017-05-17 20:29:19]

weidongg@chromium.org changed reviewers:
+ sky@chromium.org

[weidongg, 2017-05-17 20:29:19]

sky@chromium.org: Please review changes.

[weidongg, 2017-05-17 20:30:26]

On 2017/05/17 20:29:19, weidongg wrote:
> mailto:sky@chromium.org: Please review changes.

Please review changes in 
chrome/browser/chrome_notification_types.h
chrome/browser/upgrade_detector.h
chrome/browser/upgrade_detector.cc

[sky, 2017-05-18 15:50:44]

https://codereview.chromium.org/2882933002/diff/60001/chrome/browser/chrome_n...
File chrome/browser/chrome_notification_types.h (right):

https://codereview.chromium.org/2882933002/diff/60001/chrome/browser/chrome_n...
chrome/browser/chrome_notification_types.h:276: // Sent when Chrome believes an
update is available, but downloading requires
Seen comments on line 25-27. What would it take to not use notifications?

https://codereview.chromium.org/2882933002/diff/60001/chrome/common/url_const...
File chrome/common/url_constants.cc (right):

https://codereview.chromium.org/2882933002/diff/60001/chrome/common/url_const...
chrome/common/url_constants.cc:385: const char kHelpSubPage[] = "help";
Please keep sorted. Also, kChromeUIHelpHost has the same value. I take it this
is not used as the host so it's ok to have dups?

[weidongg, 2017-05-18 20:36:35]

https://codereview.chromium.org/2882933002/diff/60001/chrome/browser/chrome_n...
File chrome/browser/chrome_notification_types.h (right):

https://codereview.chromium.org/2882933002/diff/60001/chrome/browser/chrome_n...
chrome/browser/chrome_notification_types.h:276: // Sent when Chrome believes an
update is available, but downloading requires
On 2017/05/18 15:50:43, sky wrote:
> Seen comments on line 25-27. What would it take to not use notifications?

Oh, I didn't notice that. I migrate this notification to observer.

https://codereview.chromium.org/2882933002/diff/60001/chrome/common/url_const...
File chrome/common/url_constants.cc (right):

https://codereview.chromium.org/2882933002/diff/60001/chrome/common/url_const...
chrome/common/url_constants.cc:385: const char kHelpSubPage[] = "help";
On 2017/05/18 15:50:43, sky wrote:
> Please keep sorted. Also, kChromeUIHelpHost has the same value. I take it this
> is not used as the host so it's ok to have dups?

Yes, it's not used as the host.

[sky, 2017-05-18 23:41:56]

https://codereview.chromium.org/2882933002/diff/100001/chrome/browser/upgrade...
File chrome/browser/upgrade_detector.h (right):

https://codereview.chromium.org/2882933002/diff/100001/chrome/browser/upgrade...
chrome/browser/upgrade_detector.h:30: class UpgradeObserver {
Remove this to it's own file (see chromium style guide).

https://codereview.chromium.org/2882933002/diff/100001/chrome/browser/upgrade...
chrome/browser/upgrade_detector.h:32: virtual ~UpgradeObserver();
Move to protected section, also you can inline this (style guide allows it).

[weidongg, 2017-05-19 01:29:53]

https://codereview.chromium.org/2882933002/diff/100001/chrome/browser/upgrade...
File chrome/browser/upgrade_detector.h (right):

https://codereview.chromium.org/2882933002/diff/100001/chrome/browser/upgrade...
chrome/browser/upgrade_detector.h:30: class UpgradeObserver {
On 2017/05/18 23:41:56, sky wrote:
> Remove this to it's own file (see chromium style guide).

Done.

https://codereview.chromium.org/2882933002/diff/100001/chrome/browser/upgrade...
chrome/browser/upgrade_detector.h:32: virtual ~UpgradeObserver();
On 2017/05/18 23:41:56, sky wrote:
> Move to protected section, also you can inline this (style guide allows it).

Done.

[sky, 2017-05-19 14:22:28]

LGTM with the following change as well as adding the new file to the appropriate
BUILD.gn

https://codereview.chromium.org/2882933002/diff/120001/chrome/browser/upgrade...
File chrome/browser/upgrade_observer.h (right):

https://codereview.chromium.org/2882933002/diff/120001/chrome/browser/upgrade...
chrome/browser/upgrade_observer.h:15: UpgradeObserver() {}
You shouldn't need this (and if you do, it should be above the other function).

https://codereview.chromium.org/2882933002/diff/120001/chrome/browser/upgrade...
chrome/browser/upgrade_observer.h:21: DISALLOW_COPY_AND_ASSIGN(UpgradeObserver);
You shouldn't need this on a pure virtual class.

[sky, 2017-05-19 14:22:46]

Also, please file a bug to convert remaining uses to observer.

[weidongg, 2017-05-19 17:43:08]

Thanks for the review, I just filed a bug to migrate the rest of upgrade
notifications: crbug.com/724540

[weidongg, 2017-05-19 17:46:57]

The CQ bit was checked by weidongg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-19 17:47:53]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-19 20:50:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-19 20:50:16]

Dry run: This issue passed the CQ dry run.

[weidongg, 2017-05-19 20:54:27]

The CQ bit was checked by weidongg@chromium.org

[weidongg, 2017-05-19 20:54:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2882933002/#ps140001
(title: "Add file to BUILD.gn")

[commit-bot: I haz the power, 2017-05-19 20:55:31]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-19 21:05:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-19 21:05:56]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[weidongg, 2017-05-20 01:09:39]

weidongg@chromium.org changed reviewers:
+ dcheng@chromium.org

[weidongg, 2017-05-20 01:09:40]

Please review the changes in
ash/public/interfaces/system_tray.mojom

[dcheng, 2017-05-22 09:26:55]

LGTM

[weidongg, 2017-05-22 16:32:07]

The CQ bit was checked by weidongg@chromium.org

[commit-bot: I haz the power, 2017-05-22 16:32:28]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 17:47:25]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1495470727533560,
"parent_rev": "d9433138750c99a9c0a0698c2cb8ae1098cfbb31", "commit_rev":
"ef06a6f4d0434cbc277103b953ed33b12d0d0261"}

[commit-bot: I haz the power, 2017-05-22 17:47:37]

Description was changed from

==========
Add update available icon in system tray

- Show an update available icon in system tray upon receiving
NEED_PERMISSION_TO_UPDATE state from update engine.

- This icon brings user to the about Chrome OS page and prompts update
warning window with update size.

- Design doc: go/cros-cellular-updates

BUG=chromium:691108
TEST=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Add update available icon in system tray

- Show an update available icon in system tray upon receiving
NEED_PERMISSION_TO_UPDATE state from update engine.

- This icon brings user to the about Chrome OS page and prompts update
warning window with update size.

- Design doc: go/cros-cellular-updates

BUG=chromium:691108
TEST=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2882933002
Cr-Commit-Position: refs/heads/master@{#473614}
Committed:
https://chromium.googlesource.com/chromium/src/+/ef06a6f4d0434cbc277103b953ed...
==========

[commit-bot: I haz the power, 2017-05-22 17:47:39]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/ef06a6f4d0434cbc277103b953ed...