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

Issue 640305 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Defer notification window creation until UI is needed

Project Member Reported by xiy...@chromium.org, Aug 23 2016

Issue description

http://b/30864565

This issue tracks the chrome side change to defer notification window creation.
 
Cc: -yoshi@chromium.org yoshiki@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 1 2016

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

commit a21cae6cb172afe362f8a6d12f6e64ce56347b65
Author: xiyuan <xiyuan@chromium.org>
Date: Thu Sep 01 04:57:06 2016

arc: Defer notification surface creation

- Add Create/CloseNotificationWindow to NotificationsInstance;
- ArcCustomNotificationItem tracks a window count for the views and
  sends Create/CloseNotificationWindow based on the count;
- ArcCustomNotificationItem initializes the view with snapshot image;
- ArcCustomNotificationView observes surface manager and dynamically
  switch from snapshot to live surface;

BUG= 640305 ,640356
BUG=b/30864565
TEST=Manual. Custom notification works as before and CTS passes.

Review-Url: https://codereview.chromium.org/2269403004
Cr-Commit-Position: refs/heads/master@{#415887}

[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/components/arc/common/notifications.mojom
[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/components/arc/test/fake_notifications_instance.cc
[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/components/arc/test/fake_notifications_instance.h
[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/ui/arc/notification/arc_custom_notification_item.cc
[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/ui/arc/notification/arc_custom_notification_item.h
[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/ui/arc/notification/arc_custom_notification_view.h
[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/ui/arc/notification/arc_notification_item.h
[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/ui/arc/notification/arc_notification_manager.cc
[modify] https://crrev.com/a21cae6cb172afe362f8a6d12f6e64ce56347b65/ui/arc/notification/arc_notification_manager.h

Labels: M-54 Merge-Request-54 M-53 Merge-Request-53
http://b/30864565 is release blocker. Request to merge the CL in #2 to both M53 and M54.

Comment 4 by dimu@chromium.org, Sep 7 2016

Labels: -Merge-Request-53 Merge-Review-53 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M53), manual review required.

Comment 5 by dimu@chromium.org, Sep 7 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Cc: bhthompson@chromium.org
+bhthompson

This is the Chrome side change needed for CTS regression http://b/30864565. Please approve merge to M53.

Thanks
Labels: -Merge-Review-53 Merge-Approved-53
This appears to be arc specific, and is effectively release blocking so we can merge it.
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 7 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86512411d5d6937aa21a30d3dc893ee37a352df1

commit 86512411d5d6937aa21a30d3dc893ee37a352df1
Author: Xiyuan Xia <xiyuan@google.com>
Date: Wed Sep 07 21:17:11 2016

Merge "arc: Defer notification surface creation"

> - Add Create/CloseNotificationWindow to NotificationsInstance;
> - ArcCustomNotificationItem tracks a window count for the views and
>   sends Create/CloseNotificationWindow based on the count;
> - ArcCustomNotificationItem initializes the view with snapshot image;
> - ArcCustomNotificationView observes surface manager and dynamically
>   switch from snapshot to live surface;
>
> BUG= 640305 ,640356
> BUG=b/30864565
> TEST=Manual. Custom notification works as before and CTS passes.
>
> Review-Url: https://codereview.chromium.org/2269403004
> Cr-Commit-Position: refs/heads/master@{#415887}
> (cherry picked from commit a21cae6cb172afe362f8a6d12f6e64ce56347b65)

Review URL: https://codereview.chromium.org/2319893002 .

Cr-Commit-Position: refs/branch-heads/2840@{#216}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/components/arc/common/notifications.mojom
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/components/arc/test/fake_notifications_instance.cc
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/components/arc/test/fake_notifications_instance.h
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_custom_notification_item.cc
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_custom_notification_item.h
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_custom_notification_view.h
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_notification_item.h
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_notification_manager.cc
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_notification_manager.h

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 8 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39

commit 6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39
Author: Xiyuan Xia <xiyuan@google.com>
Date: Thu Sep 08 00:16:10 2016

Merge "arc: Defer notification surface creation"

> - Add Create/CloseNotificationWindow to NotificationsInstance;
> - ArcCustomNotificationItem tracks a window count for the views and
>   sends Create/CloseNotificationWindow based on the count;
> - ArcCustomNotificationItem initializes the view with snapshot image;
> - ArcCustomNotificationView observes surface manager and dynamically
>   switch from snapshot to live surface;
>
> BUG= 640305 ,640356
> BUG=b/30864565
> TEST=Manual. Custom notification works as before and CTS passes.
>
> Review-Url: https://codereview.chromium.org/2269403004
> Cr-Commit-Position: refs/heads/master@{#415887}
> (cherry picked from commit a21cae6cb172afe362f8a6d12f6e64ce56347b65)

Review URL: https://codereview.chromium.org/2323533002 .

Cr-Commit-Position: refs/branch-heads/2785@{#848}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/components/arc/common/notifications.mojom
[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/components/arc/test/fake_notifications_instance.cc
[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/components/arc/test/fake_notifications_instance.h
[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/ui/arc/notification/arc_custom_notification_item.cc
[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/ui/arc/notification/arc_custom_notification_item.h
[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/ui/arc/notification/arc_custom_notification_view.h
[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/ui/arc/notification/arc_notification_item.h
[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/ui/arc/notification/arc_notification_manager.cc
[modify] https://crrev.com/6d4a4b0e0f26513e82157bf7f0dcbe666cc41f39/ui/arc/notification/arc_notification_manager.h

All changes are merged. Will close the bug after the test bots pick up a build with the fixes and verify the CTS regression is fixed.
Status: Fixed (was: Assigned)
R55-8782.0.0 is nos passing CTS.
Status: Verified (was: Fixed)
Inner bug is verified 
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 27 2016

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

commit 86512411d5d6937aa21a30d3dc893ee37a352df1
Author: Xiyuan Xia <xiyuan@google.com>
Date: Wed Sep 07 21:17:11 2016

Merge "arc: Defer notification surface creation"

> - Add Create/CloseNotificationWindow to NotificationsInstance;
> - ArcCustomNotificationItem tracks a window count for the views and
>   sends Create/CloseNotificationWindow based on the count;
> - ArcCustomNotificationItem initializes the view with snapshot image;
> - ArcCustomNotificationView observes surface manager and dynamically
>   switch from snapshot to live surface;
>
> BUG= 640305 ,640356
> BUG=b/30864565
> TEST=Manual. Custom notification works as before and CTS passes.
>
> Review-Url: https://codereview.chromium.org/2269403004
> Cr-Commit-Position: refs/heads/master@{#415887}
> (cherry picked from commit a21cae6cb172afe362f8a6d12f6e64ce56347b65)

Review URL: https://codereview.chromium.org/2319893002 .

Cr-Commit-Position: refs/branch-heads/2840@{#216}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/components/arc/common/notifications.mojom
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/components/arc/test/fake_notifications_instance.cc
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/components/arc/test/fake_notifications_instance.h
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_custom_notification_item.cc
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_custom_notification_item.h
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_custom_notification_view.h
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_notification_item.h
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_notification_manager.cc
[modify] https://crrev.com/86512411d5d6937aa21a30d3dc893ee37a352df1/ui/arc/notification/arc_notification_manager.h

Sign in to add a comment