New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 751024 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature

Blocked on:
issue 726241
issue 738779
issue 751021
issue 760865
issue 760866
issue 760883
issue 760884

Blocking:
issue 755404



Sign in to add a comment

Port system notifications to new-style notification

Project Member Reported by tetsui@chromium.org, Aug 1 2017

Issue description

In new-style notification, system notifications have different icons. There are also elements that were not shown or existed in the old notification such as context title ("Chrome OS system" or the name of the component), or accent color (one of Normal/Warning/Critical Warning).

In order to support them, we have to modify all the system notification to use new CreateSystemNotification http://crrev.com/c/566765 .

Doc: go/cros-system-notificaton-porting-md
Mock: https://gallery.googleplex.com/projects/MCHbtQVoQ2HCZYhMpwexg3v6/files/MCEbGRW8tVgRo7OTIz9Lrt7ywMSJbxAVNpw
 
Blockedon: 751021
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cda0ef7ef22d31194aacdb6eae7f4c632f6a7e42

commit cda0ef7ef22d31194aacdb6eae7f4c632f6a7e42
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue Aug 22 04:52:59 2017

Apply system style to "ChromeVox is enabled" notification.

In new-style notification, system notification has different style from
other notifications such as web notifications.
* The context header shows the title "Chrome OS system".
* The context heaader has accent color.
This CL applies the system style to "ChromeVox is enabled" notification.

TEST=manual
BUG= 751024 

Change-Id: Ifcae548138c1f86876e32ec213db87957a1fbfe7
Reviewed-on: https://chromium-review.googlesource.com/599133
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496217}
[modify] https://crrev.com/cda0ef7ef22d31194aacdb6eae7f4c632f6a7e42/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/cda0ef7ef22d31194aacdb6eae7f4c632f6a7e42/ash/resources/vector_icons/notification_accessibility.1x.icon
[add] https://crrev.com/cda0ef7ef22d31194aacdb6eae7f4c632f6a7e42/ash/resources/vector_icons/notification_accessibility.icon
[add] https://crrev.com/cda0ef7ef22d31194aacdb6eae7f4c632f6a7e42/ash/resources/vector_icons/notification_chromevox.1x.icon
[add] https://crrev.com/cda0ef7ef22d31194aacdb6eae7f4c632f6a7e42/ash/resources/vector_icons/notification_chromevox.icon
[modify] https://crrev.com/cda0ef7ef22d31194aacdb6eae7f4c632f6a7e42/ash/system/tray_accessibility.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce10182a51b8d4c9d4b99be26a462ca2683081b0

commit ce10182a51b8d4c9d4b99be26a462ca2683081b0
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue Aug 22 07:16:56 2017

Apply system style to "CAPS LOCK is ON" notification.

In new-style notification, system notification has different style from
other notifications such as web notifications.
* The context header shows the title "Chrome OS system".
* The context header has different color.
* New MD icon is used.
This CL applies the system style to "CAPS LOCK is ON" notification.

BUG= 740813 , 751024 
TEST=manual

Change-Id: I61f6909224eb4a4f10830b636c1f5be27f4a0117
Reviewed-on: https://chromium-review.googlesource.com/598751
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496239}
[modify] https://crrev.com/ce10182a51b8d4c9d4b99be26a462ca2683081b0/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/ce10182a51b8d4c9d4b99be26a462ca2683081b0/ash/resources/vector_icons/notification_capslock.1x.icon
[add] https://crrev.com/ce10182a51b8d4c9d4b99be26a462ca2683081b0/ash/resources/vector_icons/notification_capslock.icon
[modify] https://crrev.com/ce10182a51b8d4c9d4b99be26a462ca2683081b0/ash/system/tray_caps_lock.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32af1a03265847f1a8c74fe3cab4fc9fba4876f4

commit 32af1a03265847f1a8c74fe3cab4fc9fba4876f4
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Aug 23 02:38:18 2017

Apply theme color and new icon to screenshot notification.

According to MD notification mock, screenshot notification has theme
color and new |small_image| icon. (go/qgprb)

If chrome://flags#enable-message-center-new-style-notification is
disabled, this CL has no effect.

TEST=out/Debug/browser_tests \
'--gtest_filter=ChromeScreenshotGrabberBrowserTest.*'
BUG= 751024 

Change-Id: I2287a931a966d78cee1e158bbbd6347ccafdc3bf
Reviewed-on: https://chromium-review.googlesource.com/622812
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496567}
[modify] https://crrev.com/32af1a03265847f1a8c74fe3cab4fc9fba4876f4/chrome/app/vector_icons/BUILD.gn
[add] https://crrev.com/32af1a03265847f1a8c74fe3cab4fc9fba4876f4/chrome/app/vector_icons/notification_image.1x.icon
[add] https://crrev.com/32af1a03265847f1a8c74fe3cab4fc9fba4876f4/chrome/app/vector_icons/notification_image.icon
[modify] https://crrev.com/32af1a03265847f1a8c74fe3cab4fc9fba4876f4/chrome/browser/ui/ash/chrome_screenshot_grabber.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e8f95e84d9a9890972cee9176aff8cf3cf0363da

commit e8f95e84d9a9890972cee9176aff8cf3cf0363da
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Aug 23 06:10:47 2017

Apply theme color and new icon to EOL notification.

According to MD notification mock, EOL notification has theme color and
new |small_image| icon. (go/cirvr)

If chrome://flags#enable-message-center-new-style-notification is
disabled, this CL has no effect.

TEST=manually tested on Chell device
BUG= 751024 

Change-Id: I4018d129659b1a0700ebbb695162452ce90ef8aa
Reviewed-on: https://chromium-review.googlesource.com/622509
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496595}
[modify] https://crrev.com/e8f95e84d9a9890972cee9176aff8cf3cf0363da/chrome/app/chromeos_strings.grdp
[modify] https://crrev.com/e8f95e84d9a9890972cee9176aff8cf3cf0363da/chrome/app/vector_icons/BUILD.gn
[add] https://crrev.com/e8f95e84d9a9890972cee9176aff8cf3cf0363da/chrome/app/vector_icons/notification_end_of_support.icon
[modify] https://crrev.com/e8f95e84d9a9890972cee9176aff8cf3cf0363da/chrome/browser/chromeos/eol_notification.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d9661b59ca265b0ec2bbfe8673ef27096555485

commit 1d9661b59ca265b0ec2bbfe8673ef27096555485
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Aug 24 02:35:35 2017

Add utility function for new style notification support.

During transition to new style notification, system notification should
check IsNewStyleNotificationEnabled() and switch between
Notification::CreateSystemNotification and Notification constructor,
so the transition would not cause the regression when the flag is
diabled.
Most of the arguments are common between CreateSystemNotification and
the constructor, so it is verbose to check the flag at each call site.
This CL adds ash::system_notifier::CreateSystemNotification which checks
the flags internally.

TEST=manual
BUG= 751024 

Change-Id: I46b719cd3fb3f336070c469e2dfce9820e379c10
Reviewed-on: https://chromium-review.googlesource.com/627970
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496926}
[modify] https://crrev.com/1d9661b59ca265b0ec2bbfe8673ef27096555485/ash/system/system_notifier.cc
[modify] https://crrev.com/1d9661b59ca265b0ec2bbfe8673ef27096555485/ash/system/system_notifier.h

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7b3fbbf02df8cfe843913504c6be1e7e047d20a

commit c7b3fbbf02df8cfe843913504c6be1e7e047d20a
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Aug 24 16:08:40 2017

Apply system style to "Low-power charger connected" notification.

In new-style notification system notification has different style from
other notifications such as web notifications.
* The context header shous the title "Chrome OS system".
* The context header has accent color.
* New MD icon is used.
This CL applies the system style to "Low-power charger connected"
notification.

TEST=manual
BUG= 751024 

Change-Id: I94c48123fec644d28ba044ca22a5bb17feb737fb
Reviewed-on: https://chromium-review.googlesource.com/603229
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497068}
[modify] https://crrev.com/c7b3fbbf02df8cfe843913504c6be1e7e047d20a/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/c7b3fbbf02df8cfe843913504c6be1e7e047d20a/ash/resources/vector_icons/notification_low_power_battery.icon
[modify] https://crrev.com/c7b3fbbf02df8cfe843913504c6be1e7e047d20a/ash/system/power/tray_power.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d3f866e9ad4049bc0e1c1c6535db3725d85467c

commit 2d3f866e9ad4049bc0e1c1c6535db3725d85467c
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Fri Aug 25 02:00:01 2017

Apply system style to Bluetooth notification.

In new-style notification system notification has different style from
other notifications such as web notifications.
* The context header shous the title "Chrome OS system".
* The context header has accent color.
* New MD icon is used.
This CL applies the system style to Bluetooth notification.

If chrome://flags#enable-message-center-new-style-notification is
disabled, this CL has no effect.

TEST=manually tested on chell device
BUG= 751024 

Change-Id: Ib3293761013b08c0c6b817b6ad04ea910ed72120
Reviewed-on: https://chromium-review.googlesource.com/604976
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497297}
[modify] https://crrev.com/2d3f866e9ad4049bc0e1c1c6535db3725d85467c/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/2d3f866e9ad4049bc0e1c1c6535db3725d85467c/ash/resources/vector_icons/notification_bluetooth.icon
[modify] https://crrev.com/2d3f866e9ad4049bc0e1c1c6535db3725d85467c/ash/system/bluetooth/bluetooth_notification_controller.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c21315070ca9e46d8aeaef74cde010240a7d4ff

commit 2c21315070ca9e46d8aeaef74cde010240a7d4ff
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Mon Aug 28 03:08:43 2017

Apply system style to battery low notification.

In new-style notification system notification has different style from
other notifications such as web notifications.
* The context header shous the title "Chrome OS system".
* The context header has accent color.
* New MD icon is used.
This CL applies the system style to battery low notification.

TEST=manual
BUG= 751024 

Change-Id: I8921aed02aa4bb22a845bb2088abf071c83d594f
Reviewed-on: https://chromium-review.googlesource.com/623042
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497690}
[modify] https://crrev.com/2c21315070ca9e46d8aeaef74cde010240a7d4ff/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/2c21315070ca9e46d8aeaef74cde010240a7d4ff/ash/resources/vector_icons/notification_battery_critical.icon
[add] https://crrev.com/2c21315070ca9e46d8aeaef74cde010240a7d4ff/ash/resources/vector_icons/notification_battery_fluctuating.icon
[add] https://crrev.com/2c21315070ca9e46d8aeaef74cde010240a7d4ff/ash/resources/vector_icons/notification_battery_low.icon
[modify] https://crrev.com/2c21315070ca9e46d8aeaef74cde010240a7d4ff/ash/system/power/battery_notification.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/745c2adf7ed296ae2033c9676c4e3f33ad0774e5

commit 745c2adf7ed296ae2033c9676c4e3f33ad0774e5
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Mon Aug 28 04:30:54 2017

Apply system style to shortcut changed notification.

In new-style notification system notification has different style from
other notifications such as web notifications.
This CL makes following changes to shortcut changed notification.
* New MD icon is added.
* The notification title is added.

TEST=manually tested on chell device
BUG= 751024 ,758269

Change-Id: Ie1649d9c80d53e4d031ddd3b45b8a40efae7cef3
Reviewed-on: https://chromium-review.googlesource.com/634774
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497695}
[modify] https://crrev.com/745c2adf7ed296ae2033c9676c4e3f33ad0774e5/ash/accelerators/accelerator_controller.cc
[modify] https://crrev.com/745c2adf7ed296ae2033c9676c4e3f33ad0774e5/ash/ash_strings.grd
[modify] https://crrev.com/745c2adf7ed296ae2033c9676c4e3f33ad0774e5/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/745c2adf7ed296ae2033c9676c4e3f33ad0774e5/ash/resources/vector_icons/notification_settings.icon

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c300ce1b49186019da200aa28716ca98b42ac711

commit c300ce1b49186019da200aa28716ca98b42ac711
Author: Henrik Andreasson <henrika@chromium.org>
Date: Mon Aug 28 08:56:34 2017

Revert "Apply system style to shortcut changed notification."

This reverts commit 745c2adf7ed296ae2033c9676c4e3f33ad0774e5.

Reason for revert: breaks  Linux Chromium OS ASan LSan Tests (1)

https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/23305

https://findit-for-me.appspot.com/waterfall/build-failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/23305

Original change's description:
> Apply system style to shortcut changed notification.
> 
> In new-style notification system notification has different style from
> other notifications such as web notifications.
> This CL makes following changes to shortcut changed notification.
> * New MD icon is added.
> * The notification title is added.
> 
> TEST=manually tested on chell device
> BUG= 751024 ,758269
> 
> Change-Id: Ie1649d9c80d53e4d031ddd3b45b8a40efae7cef3
> Reviewed-on: https://chromium-review.googlesource.com/634774
> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#497695}

TBR=jamescook@chromium.org,tetsui@chromium.org

Change-Id: Ib11ef6266c92757afe45863d752e08848936da36
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  751024 , 758269
Reviewed-on: https://chromium-review.googlesource.com/636424
Reviewed-by: Henrik Andreasson <henrika@chromium.org>
Commit-Queue: Henrik Andreasson <henrika@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497719}
[modify] https://crrev.com/c300ce1b49186019da200aa28716ca98b42ac711/ash/accelerators/accelerator_controller.cc
[modify] https://crrev.com/c300ce1b49186019da200aa28716ca98b42ac711/ash/ash_strings.grd
[modify] https://crrev.com/c300ce1b49186019da200aa28716ca98b42ac711/ash/resources/vector_icons/BUILD.gn
[delete] https://crrev.com/890685b158cc617d78039205988dc3ac04c2e29d/ash/resources/vector_icons/notification_settings.icon

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fd8f9c1e1b02dc48266834f0bf141bb33edf754e

commit fd8f9c1e1b02dc48266834f0bf141bb33edf754e
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue Aug 29 05:49:33 2017

Apply system style to High Contrast Mode notification.

In new-style notification system notification has different style from
other notifications such as web notifications.
* The context header shous the title "Chrome OS system".
* The context header has accent color.
* New MD icon is used.
This CL applies the system style to High Contrast Mode notification.

TEST=manual
BUG= 751024 

Change-Id: I7ab022e5b7fcd7e2807ca5327e196c1e2f6b3e84
Reviewed-on: https://chromium-review.googlesource.com/637058
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498030}
[modify] https://crrev.com/fd8f9c1e1b02dc48266834f0bf141bb33edf754e/ash/accelerators/accelerator_controller.cc
[modify] https://crrev.com/fd8f9c1e1b02dc48266834f0bf141bb33edf754e/ash/accelerators/accelerator_controller_unittest.cc
[modify] https://crrev.com/fd8f9c1e1b02dc48266834f0bf141bb33edf754e/ash/ash_strings.grd

Project Member

Comment 13 by bugdroid1@chromium.org, Aug 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b550b9b59e107fdf3e25f88bc30e37f978ebbe45

commit b550b9b59e107fdf3e25f88bc30e37f978ebbe45
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue Aug 29 06:30:13 2017

Apply system style to session ending notification.

In new style notification, system notification has different style from
other notifications such as web notifications.
* The context header shows the title "Chrome OS system".
* The context header has accent color.
* New MD icon is used.
This CL applies the system style to "Session ends in ..." notification.

TEST=manual
BUG= 751024 

Change-Id: I7124115bb330e862b511e89def920732380ac4d1
Reviewed-on: https://chromium-review.googlesource.com/623022
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498033}
[modify] https://crrev.com/b550b9b59e107fdf3e25f88bc30e37f978ebbe45/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/b550b9b59e107fdf3e25f88bc30e37f978ebbe45/ash/resources/vector_icons/notification_timer.icon
[modify] https://crrev.com/b550b9b59e107fdf3e25f88bc30e37f978ebbe45/ash/system/session/tray_session_length_limit.cc
[modify] https://crrev.com/b550b9b59e107fdf3e25f88bc30e37f978ebbe45/ash/system/session/tray_session_length_limit_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Aug 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39754e74f0335375d4af97c5cdb11b8cd92308f0

commit 39754e74f0335375d4af97c5cdb11b8cd92308f0
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Aug 30 03:21:36 2017

Reland "Apply system style to shortcut changed notification."

This is a reland of 745c2adf7ed296ae2033c9676c4e3f33ad0774e5

Revert CL: https://crrev.com/c/chromium/src/+/636424
Reason for revert: breaks  Linux Chromium OS ASan LSan Tests (1)

Change from original CL (which is patch set 1):
* Remove all notifications at the end of
  AcceleratorControllerTest.GlobalAccelerators,
  so that the remaining notification objects are not recognized as leak.

Original change's description:
> Apply system style to shortcut changed notification.
>
> In new-style notification system notification has different style from
> other notifications such as web notifications.
> This CL makes following changes to shortcut changed notification.
> * New MD icon is added.
> * The notification title is added.
>
> TEST=manually tested on chell device
> BUG= 751024 ,758269
>
> Change-Id: Ie1649d9c80d53e4d031ddd3b45b8a40efae7cef3
> Reviewed-on: https://chromium-review.googlesource.com/634774
> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#497695}

Bug:  751024 , 758269
Change-Id: Icdf1662acbdfc6336010a8b1d652c87fcc23c689
Reviewed-on: https://chromium-review.googlesource.com/637650
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498358}
[modify] https://crrev.com/39754e74f0335375d4af97c5cdb11b8cd92308f0/ash/accelerators/accelerator_controller.cc
[modify] https://crrev.com/39754e74f0335375d4af97c5cdb11b8cd92308f0/ash/accelerators/accelerator_controller_unittest.cc
[modify] https://crrev.com/39754e74f0335375d4af97c5cdb11b8cd92308f0/ash/ash_strings.grd
[modify] https://crrev.com/39754e74f0335375d4af97c5cdb11b8cd92308f0/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/39754e74f0335375d4af97c5cdb11b8cd92308f0/ash/resources/vector_icons/notification_settings.icon

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4109be8e22ed84cf58e297d04974bdc03ec80c0

commit b4109be8e22ed84cf58e297d04974bdc03ec80c0
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Aug 30 05:20:47 2017

Apply system style to WiFi notification.

In new-style notification system notification has different style from
other notifications such as web notifications.
* The context header shous the title "Chrome OS system".
* The context header has accent color.
* New MD icon is used.
This CL applies the system style to WiFi notification.

TEST=manual
BUG= 751024 

Change-Id: Idae175f3e413851b2d2c4729e429088007551387
Reviewed-on: https://chromium-review.googlesource.com/603387
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498370}
[modify] https://crrev.com/b4109be8e22ed84cf58e297d04974bdc03ec80c0/ash/system/system_notifier.h
[modify] https://crrev.com/b4109be8e22ed84cf58e297d04974bdc03ec80c0/chrome/app/vector_icons/BUILD.gn
[add] https://crrev.com/b4109be8e22ed84cf58e297d04974bdc03ec80c0/chrome/app/vector_icons/notification_captive_portal.icon
[modify] https://crrev.com/b4109be8e22ed84cf58e297d04974bdc03ec80c0/chrome/browser/chromeos/net/network_portal_notification_controller.cc

Blockedon: 760865
Blockedon: 760866
Blocking: 755404
Blockedon: 760883
Blockedon: 760884
Status: Started (was: Assigned)
Project Member

Comment 22 by bugdroid1@chromium.org, Sep 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54ec434578c9b9352fabf98cb7a18633f42341c7

commit 54ec434578c9b9352fabf98cb7a18633f42341c7
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Sep 06 10:23:47 2017

Add theme color and context title to Sign-In error notification.

- In MD notification, system notifications should have |accent_color|.
- |display_source| was not specified. This is used as the context title
  in MD notification.

Sceenshots:
- Before this CL: https://screenshot.googleplex.com/v8xnbtubgmY
- After this CL: https://screenshot.googleplex.com/0Xk6jdbuvC2

TEST=manual
1. Login on the device
2. Revoke login from https://myaccount.google.com/device-activity
3. The notification appears.
BUG= 751024 

Change-Id: I7881a3028f91d800b64b0b889718c2d1d57d07fb
Reviewed-on: https://chromium-review.googlesource.com/648891
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499924}
[modify] https://crrev.com/54ec434578c9b9352fabf98cb7a18633f42341c7/chrome/app/generated_resources.grd
[modify] https://crrev.com/54ec434578c9b9352fabf98cb7a18633f42341c7/chrome/browser/signin/signin_error_notifier_ash.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/988f649211af429f096a3f9c5b19587081b1c4c1

commit 988f649211af429f096a3f9c5b19587081b1c4c1
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Mon Sep 11 02:46:51 2017

Add theme color and context title to AD Sign-In error notification.

This change is equivalent change of https://crrev.com/c/648891 for
Active Directory login.

- With recent notification design change, now notifications generated
  by system should have |accent_color|.
- |display_source|, which is used as the context title in recent design,
  was not specified. Previously the text was not shown in this case,
  but with recent design change, |display_source| is shown to users for
  all the notifications.

TEST=manual
BUG= 751024 

Change-Id: I24b194d22bec6f9de6fb70657da67a88b2cf2467
Reviewed-on: https://chromium-review.googlesource.com/657138
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500831}
[modify] https://crrev.com/988f649211af429f096a3f9c5b19587081b1c4c1/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f79325e1a5a3876901899568acaa956606380b9b

commit f79325e1a5a3876901899568acaa956606380b9b
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Mon Sep 11 05:48:27 2017

Apply system style to screen share notification.

In new-style notification, system notification has different style from
other notifications such as web notification.
This CL applies the system style to screen share notification.
* The context header and action buttons have accent color.
* New material design icon is added.
* The title text is added.

TEST=manual
BUG= 751024 

Change-Id: I43c623e6fe5a960289424500292ed75c2d63f2cb
Reviewed-on: https://chromium-review.googlesource.com/656782
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500846}
[modify] https://crrev.com/f79325e1a5a3876901899568acaa956606380b9b/ash/ash_strings.grd
[modify] https://crrev.com/f79325e1a5a3876901899568acaa956606380b9b/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/f79325e1a5a3876901899568acaa956606380b9b/ash/resources/vector_icons/notification_screenshare.1x.icon
[add] https://crrev.com/f79325e1a5a3876901899568acaa956606380b9b/ash/resources/vector_icons/notification_screenshare.icon
[modify] https://crrev.com/f79325e1a5a3876901899568acaa956606380b9b/ash/system/screen_security/screen_share_tray_item.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71fc2d50ad90d4bf7d156c7f5646eb70cf4ac3a7

commit 71fc2d50ad90d4bf7d156c7f5646eb70cf4ac3a7
Author: Ben Wells <benwells@chromium.org>
Date: Mon Sep 11 07:54:51 2017

Revert "Apply system style to screen share notification."

This reverts commit f79325e1a5a3876901899568acaa956606380b9b.

Reason for revert: seems to have caused a crash on the bots.

First failure here: https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/30061

Sample crash log:
[ RUN      ] TrySwitchingUserTest.BothActiveAccepted
Received signal 11 <unknown> 000000000000
#0 0x7f0dcb176b5d base::debug::StackTrace::StackTrace()
#1 0x7f0dcb1750dc base::debug::StackTrace::StackTrace()
#2 0x7f0dcb176557 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#3 0x7f0dcebdd330 <unknown>
#4 0x7f0dbedd60b3 ash::Shelf::IsHorizontalAlignment()
#5 0x7f0dbef80c92 ash::WebNotificationItem::AnimationProgressed()
#6 0x7f0dca7357b1 gfx::LinearAnimation::Step()
#7 0x7f0dca730799 gfx::AnimationContainer::Run()
#8 0x7f0dca734f3d _ZN4base8internal13FunctorTraitsIMN3gfx18AnimationContainerEFvvEvE6InvokeIPS3_JEEEvS5_OT_DpOT0_
.......

Original change's description:
> Apply system style to screen share notification.
> 
> In new-style notification, system notification has different style from
> other notifications such as web notification.
> This CL applies the system style to screen share notification.
> * The context header and action buttons have accent color.
> * New material design icon is added.
> * The title text is added.
> 
> TEST=manual
> BUG= 751024 
> 
> Change-Id: I43c623e6fe5a960289424500292ed75c2d63f2cb
> Reviewed-on: https://chromium-review.googlesource.com/656782
> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#500846}

TBR=jamescook@chromium.org,tetsui@chromium.org

Change-Id: Ie0fcf633c7610b1ae1961d387d0aeb910147878a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  751024 
Reviewed-on: https://chromium-review.googlesource.com/658583
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500856}
[modify] https://crrev.com/71fc2d50ad90d4bf7d156c7f5646eb70cf4ac3a7/ash/ash_strings.grd
[modify] https://crrev.com/71fc2d50ad90d4bf7d156c7f5646eb70cf4ac3a7/ash/resources/vector_icons/BUILD.gn
[delete] https://crrev.com/4a0a2f1cea01e9f212e75b3d32f8469be70ad54d/ash/resources/vector_icons/notification_screenshare.1x.icon
[delete] https://crrev.com/4a0a2f1cea01e9f212e75b3d32f8469be70ad54d/ash/resources/vector_icons/notification_screenshare.icon
[modify] https://crrev.com/71fc2d50ad90d4bf7d156c7f5646eb70cf4ac3a7/ash/system/screen_security/screen_share_tray_item.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3666eb7dc3d26e0e7eed74c8644b26c7f0bf2468

commit 3666eb7dc3d26e0e7eed74c8644b26c7f0bf2468
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue Sep 12 04:52:11 2017

Reland "Apply system style to screen share notification."

This is a reland of f79325e1a5a3876901899568acaa956606380b9b

Change from the original CL (which is patch set #1):
* Disable tray animation in TrySwitchingUserTest. The animation was
  not triggered before because |small_image| was not set.

Original change's description:
> Apply system style to screen share notification.
> 
> In new-style notification, system notification has different style from
> other notifications such as web notification.
> This CL applies the system style to screen share notification.
> * The context header and action buttons have accent color.
> * New material design icon is added.
> * The title text is added.
> 
> TEST=manual
> BUG= 751024 
> 
> Change-Id: I43c623e6fe5a960289424500292ed75c2d63f2cb
> Reviewed-on: https://chromium-review.googlesource.com/656782
> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#500846}

Bug:  751024 
Change-Id: If2b9b7779fb68be80a45d4584c9dfacadd0c2f57
Reviewed-on: https://chromium-review.googlesource.com/658681
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501178}
[modify] https://crrev.com/3666eb7dc3d26e0e7eed74c8644b26c7f0bf2468/ash/ash_strings.grd
[modify] https://crrev.com/3666eb7dc3d26e0e7eed74c8644b26c7f0bf2468/ash/resources/vector_icons/BUILD.gn
[add] https://crrev.com/3666eb7dc3d26e0e7eed74c8644b26c7f0bf2468/ash/resources/vector_icons/notification_screenshare.1x.icon
[add] https://crrev.com/3666eb7dc3d26e0e7eed74c8644b26c7f0bf2468/ash/resources/vector_icons/notification_screenshare.icon
[modify] https://crrev.com/3666eb7dc3d26e0e7eed74c8644b26c7f0bf2468/ash/system/screen_security/screen_share_tray_item.cc
[modify] https://crrev.com/3666eb7dc3d26e0e7eed74c8644b26c7f0bf2468/chrome/browser/ui/ash/multi_user/user_switch_util_unittest.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Sep 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8eb7f17f0c3a68fa73311f1e5dc2d7b943fa0427

commit 8eb7f17f0c3a68fa73311f1e5dc2d7b943fa0427
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Fri Sep 15 04:19:40 2017

Apply system style to screen capture notification.

In new-style notification, system notification has different style from
other notifications such as web notification.
This CL applies the system style to screen capture notification.
* The context header and action buttons have accent color.
* New material design icon is added.
* The title text is added.
  * The icon and the title are same as screen share notification.
    https://crrev.com/c/656782

TEST=manual
BUG= 751024 

Change-Id: I86c55bcf5256ca9caf3663c5217bb28cc4d425ab
Reviewed-on: https://chromium-review.googlesource.com/666102
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502155}
[modify] https://crrev.com/8eb7f17f0c3a68fa73311f1e5dc2d7b943fa0427/ash/system/screen_security/screen_capture_tray_item.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Sep 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6df32faa4cd95790ef1eb498fe221dad5f72a533

commit 6df32faa4cd95790ef1eb498fe221dad5f72a533
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Fri Sep 15 07:48:07 2017

Apply system style to disk low notification.

In new style notification, system notifications have different style
from other notifications such as web notifications.
* The context header shows the title "Chrome OS system".
* The context header has accent color, depending on severity of the
  notification.
* New material design icon is used.

TEST=manual
BUG= 751024 

Change-Id: I36d2c5d43bf85b0f91828b48918f3066cf0b6eb3
Reviewed-on: https://chromium-review.googlesource.com/658014
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502191}
[modify] https://crrev.com/6df32faa4cd95790ef1eb498fe221dad5f72a533/chrome/app/vector_icons/BUILD.gn
[add] https://crrev.com/6df32faa4cd95790ef1eb498fe221dad5f72a533/chrome/app/vector_icons/notification_storage_full.icon
[modify] https://crrev.com/6df32faa4cd95790ef1eb498fe221dad5f72a533/chrome/browser/chromeos/ui/low_disk_notification.cc

Status: Fixed (was: Started)
In general, this is finished. For remaining tasks, I would like to do on another issue.

Comment 30 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 31 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment