Fix crash of update-over-cellular icon

Fix crash of update-over-cellular icon

The CL disables update-over-cellular icon in system tray in locked
and login screen, as the icon will bring user to settings page which
is not available yet.

BUG=729202
Review-Url: https://codereview.chromium.org/2915383002
Cr-Commit-Position: refs/heads/master@{#477015}
Committed: https://chromium.googlesource.com/chromium/src/+/0320b2c46526df82b647a224a995642dfad79f7b

[weidongg, 2017-06-02 22:52:39]

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

[xiyuan, 2017-06-02 23:06:23]

Update engine should not do anything until oobe is marked as complete. How did
you get the icon to show up?

[weidongg, 2017-06-03 00:14:56]

Description was changed from

==========
Fix crash of update-over-cellular icon

The CL avoids showing update-over-cellular icon in system tray in OOBE,
as the icon will bring user to settings page which is not loaded yet.
The icon should only show up after OOBE.

BUG=729202
==========

to

==========
Fix crash of update-over-cellular icon

The CL disables update-over-cellular icon in system tray in locked
and login screen, as the icon will bring user to settings page which
is not available yet.

BUG=729202
==========

[weidongg, 2017-06-03 00:17:38]

On 2017/06/02 23:06:23, xiyuan wrote:
> Update engine should not do anything until oobe is marked as complete. How did
> you get the icon to show up?

Sorry, I confused OOBE with login and locked screen. I uploaded
a new patch set.

[xiyuan, 2017-06-05 16:12:52]

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

https://codereview.chromium.org/2915383002/diff/20001/ash/system/update/tray_...
ash/system/update/tray_update.cc:70: explicit UpdateView(TrayUpdate* owner,
LoginStatus status)
nit: "explicit" can be removed since this is no longer a single arg ctor.

https://codereview.chromium.org/2915383002/diff/20001/ash/system/update/tray_...
ash/system/update/tray_update.cc:96: status == LoginStatus::NOT_LOGGED_IN) {
Should we call SessionController::ShouldEnableSettings to be consistent with the
settings button?

https://codereview.chromium.org/2915383002/diff/20001/ash/system/update/tray_...
ash/system/update/tray_update.cc:100: update_label_->SetEnabled(false);
Does this only apply to cellular update available state? Should we apply it for
all update cases?

[weidongg, 2017-06-05 16:56:52]

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

https://codereview.chromium.org/2915383002/diff/20001/ash/system/update/tray_...
ash/system/update/tray_update.cc:70: explicit UpdateView(TrayUpdate* owner,
LoginStatus status)
On 2017/06/05 16:12:52, xiyuan wrote:
> nit: "explicit" can be removed since this is no longer a single arg ctor.

Removed the second parameter.

https://codereview.chromium.org/2915383002/diff/20001/ash/system/update/tray_...
ash/system/update/tray_update.cc:96: status == LoginStatus::NOT_LOGGED_IN) {
On 2017/06/05 16:12:52, xiyuan wrote:
> Should we call SessionController::ShouldEnableSettings to be consistent with
the
> settings button?

Aha, yes, good to know this api. Thanks.

https://codereview.chromium.org/2915383002/diff/20001/ash/system/update/tray_...
ash/system/update/tray_update.cc:100: update_label_->SetEnabled(false);
On 2017/06/05 16:12:52, xiyuan wrote:
> Does this only apply to cellular update available state? Should we apply it
for
> all update cases?

The action of clicking in other update cases is rebooting, so I think we  only
need to disable cellular update case.

[xiyuan, 2017-06-05 16:58:18]

lgtm

[weidongg, 2017-06-05 16:59:26]

The CQ bit was checked by weidongg@chromium.org

[commit-bot: I haz the power, 2017-06-05 16:59:41]

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-06-05 17:08:51]

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

[commit-bot: I haz the power, 2017-06-05 17:08:52]

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-06-05 17:36:12]

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

[weidongg, 2017-06-05 17:36:13]

stevenjb@chromium.org: Please review changes in 

ash/system/update/tray_update.cc

[stevenjb, 2017-06-05 17:43:55]

lgtm

[weidongg, 2017-06-05 17:44:12]

The CQ bit was checked by weidongg@chromium.org

[commit-bot: I haz the power, 2017-06-05 17:44:36]

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-06-05 17:50:04]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496684652589690,
"parent_rev": "63fbee2219010fc20b73e0dfb9a378d43c9332fa", "commit_rev":
"0320b2c46526df82b647a224a995642dfad79f7b"}

[commit-bot: I haz the power, 2017-06-05 17:50:13]

Description was changed from

==========
Fix crash of update-over-cellular icon

The CL disables update-over-cellular icon in system tray in locked
and login screen, as the icon will bring user to settings page which
is not available yet.

BUG=729202
==========

to

==========
Fix crash of update-over-cellular icon

The CL disables update-over-cellular icon in system tray in locked
and login screen, as the icon will bring user to settings page which
is not available yet.

BUG=729202

Review-Url: https://codereview.chromium.org/2915383002
Cr-Commit-Position: refs/heads/master@{#477015}
Committed:
https://chromium.googlesource.com/chromium/src/+/0320b2c46526df82b647a224a995...
==========

[commit-bot: I haz the power, 2017-06-05 17:50:14]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/0320b2c46526df82b647a224a995...