Hide icons in message center on lock screen

Hide icons in message center on lock screen

This patch removes all the icons in the message center when the screen is locked. This patch also sets the height of the button bar to keep the height even when all the icons are invisible in the message center.

BUG=674410
TEST=manually

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

[yoshiki, 2017-01-10 13:55:20]

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

[commit-bot: I haz the power, 2017-01-10 13:55:33]

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-01-10 14:51:16]

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

[commit-bot: I haz the power, 2017-01-10 14:51:16]

Dry run: This issue passed the CQ dry run.

[yoshiki, 2017-01-10 17:09:53]

Description was changed from

==========
wip

.


.

BUG=674410
==========

to

==========
Hide icons in message center on lock screen

This patch removes all the icons in the message center when the screen is
locked. This patch also sets the height of the button bar to keep the height
even when all the icons are invisible in the message center.

BUG=674410
TEST=manually
==========

[yoshiki, 2017-01-10 17:10:46]

yoshiki@chromium.org changed reviewers:
+ dewittj@chromium.org

[yoshiki, 2017-01-10 17:10:57]

Justin, PTAL. Thanks.

[dewittj, 2017-01-12 18:29:57]

can you please add a test? (perhaps to message_center_view_unittest)

otherwise code lgtm

https://codereview.chromium.org/2622603003/diff/1/ui/message_center/views/mes...
File ui/message_center/views/message_center_view.cc (right):

https://codereview.chromium.org/2622603003/diff/1/ui/message_center/views/mes...
ui/message_center/views/message_center_view.cc:628: if (is_locked_)
nit: This seems somewwhat fragile to future updates as-is, perhaps you could do
a small helper function like:

if (!is_locked_)
  EnableCloseAllIfAppropriate();

then the early return would not be necessary.

[yoshiki, 2017-01-18 04:36:13]

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

[commit-bot: I haz the power, 2017-01-18 04:36:24]

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-01-18 05:49:00]

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

[commit-bot: I haz the power, 2017-01-18 05:49:01]

Dry run: This issue passed the CQ dry run.

[yoshiki, 2017-01-18 09:17:41]

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

[yoshiki, 2017-01-18 09:17:41]

sky@: could you take a look at the changes under ui/layout? Thanks

[sky, 2017-01-18 16:55:03]

https://codereview.chromium.org/2622603003/diff/20001/ui/views/layout/grid_la...
File ui/views/layout/grid_layout.h (right):

https://codereview.chromium.org/2622603003/diff/20001/ui/views/layout/grid_la...
ui/views/layout/grid_layout.h:136: void StartRowWithHeight(float
vertical_resize, int column_set_id, int height);
I think I would rather add a default argument to StartRow, e.g.:

void StartRow(float, int, int height = 0)

 WDYT?

[yoshiki, 2017-01-31 17:39:50]

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

[commit-bot: I haz the power, 2017-01-31 17:40:23]

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-01-31 17:53:40]

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

[commit-bot: I haz the power, 2017-01-31 17:53:41]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yoshiki, 2017-01-31 18:21:58]

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

[commit-bot: I haz the power, 2017-01-31 18:22:39]

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-01-31 18:36:03]

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

[commit-bot: I haz the power, 2017-01-31 18:36:04]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yoshiki, 2017-01-31 18:50:00]

Patchset #4 (id:60001) has been deleted

[yoshiki, 2017-01-31 18:50:05]

Patchset #3 (id:40001) has been deleted

[yoshiki, 2017-02-01 03:57:03]

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

[commit-bot: I haz the power, 2017-02-01 03:57:11]

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-02-01 04:46:59]

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

[commit-bot: I haz the power, 2017-02-01 04:47:00]

Dry run: This issue passed the CQ dry run.

[yoshiki, 2017-02-01 09:47:43]

Sky, PTAL. Thanks

https://codereview.chromium.org/2622603003/diff/20001/ui/views/layout/grid_la...
File ui/views/layout/grid_layout.h (right):

https://codereview.chromium.org/2622603003/diff/20001/ui/views/layout/grid_la...
ui/views/layout/grid_layout.h:136: void StartRowWithHeight(float
vertical_resize, int column_set_id, int height);
On 2017/01/18 16:55:02, sky wrote:
> I think I would rather add a default argument to StartRow, e.g.:
> 
> void StartRow(float, int, int height = 0)
> 
>  WDYT?

Done.

[sky, 2017-02-01 16:33:51]

LGTM

https://codereview.chromium.org/2622603003/diff/80001/ui/views/layout/grid_la...
File ui/views/layout/grid_layout.h (right):

https://codereview.chromium.org/2622603003/diff/80001/ui/views/layout/grid_la...
ui/views/layout/grid_layout.h:132: // Starts a new row with the specified column
set.
Document what |height| means, and what 0 means in this context.

[yoshiki, 2017-02-01 17:58:41]

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

[commit-bot: I haz the power, 2017-02-01 17:58:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yoshiki, 2017-02-01 18:01:06]

Thanks!

https://codereview.chromium.org/2622603003/diff/80001/ui/views/layout/grid_la...
File ui/views/layout/grid_layout.h (right):

https://codereview.chromium.org/2622603003/diff/80001/ui/views/layout/grid_la...
ui/views/layout/grid_layout.h:132: // Starts a new row with the specified column
set.
On 2017/02/01 16:33:51, sky wrote:
> Document what |height| means, and what 0 means in this context.

Done.

[commit-bot: I haz the power, 2017-02-01 18:54:00]

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

[commit-bot: I haz the power, 2017-02-01 18:54:01]

Dry run: This issue passed the CQ dry run.

[yoshiki, 2017-02-02 01:39:41]

The CQ bit was checked by yoshiki@chromium.org

[yoshiki, 2017-02-02 01:39:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
dewittj@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2622603003/#ps100001
(title: "Mentioned about height parameter")

[commit-bot: I haz the power, 2017-02-02 01:40:01]

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-02-02 01:49:04]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485999581346540,
"parent_rev": "9140e1983161fb820586389b1a77596fef4f835a", "commit_rev":
"bc9a6161faa2b8261a38df8f0d640cb42a7e26d0"}

[commit-bot: I haz the power, 2017-02-02 01:49:44]

Description was changed from

==========
Hide icons in message center on lock screen

This patch removes all the icons in the message center when the screen is
locked. This patch also sets the height of the button bar to keep the height
even when all the icons are invisible in the message center.

BUG=674410
TEST=manually
==========

to

==========
Hide icons in message center on lock screen

This patch removes all the icons in the message center when the screen is
locked. This patch also sets the height of the button bar to keep the height
even when all the icons are invisible in the message center.

BUG=674410
TEST=manually

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

[commit-bot: I haz the power, 2017-02-02 01:49:47]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/bc9a6161faa2b8261a38df8f0d64...