Move the close button out of widget |
||||||
Issue descriptionCurrently the control buttons (close and setting buttons) are placed in a widget. That is to show buttons above NativeViewHost. But using widget causes some issues (b/62771497) and prevents us from sharing the code between chromeos and other platforms. So, let's remove it and make it a normal view.
,
Jul 24 2017
Let me request merging. This should fix crashes: https://crash.corp.google.com/browse?q=product.Version%20like%20%2761.%25%27%20OR%20product.Version%20like%20%2760.%25%27%20AND%20product.name%3D%27Chrome_ChromeOS%27%20OMIT%20RECORD%20IF%20SUM(CrashedStackTrace.StackFrame.FunctionName%3D%27ui%3A%3APropertyHandler%3A%3AGetPropertyInternal(void%20const*%2C%20long)%20const%27)%20%3D%200%20OR%20SUM(CrashedStackTrace.StackFrame.FunctionName%20like%20%27arc%3A%3A%25%27)%20%3D%200&ignore_case=false&enable_rewrite=false&omit_field_name=&omit_field_value=&omit_field_opt=
,
Jul 25 2017
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b14f20f949abe14f4317b525f279e95004512b4f commit b14f20f949abe14f4317b525f279e95004512b4f Author: Yoshiki Iguchi <yoshiki@chromium.org> Date: Tue Jul 25 10:28:24 2017 Revert "Move the control buttons from dedicate widget to view" This reverts commit 084388c254c2cde590a4e3df8601ac1e1ed84c88. Reason for revert: This change breaks some layout in notification. Original change's description: > Move the control buttons from dedicate widget to view > > Previously, the buttons are on the dedicate widget to show them above > the ARC notification (NativeViewHost). This CL moves them to a view. > > In addition, this CL adds a hacks for ARC notification which manipulates > the hierarycy of layers to show the buttons at top most. > > Bug: b/62771497 > Bug: 717455 > Bug: 747217 > Change-Id: I876c653e01d4a4237055bd4f763aa55017ba38ef > Reviewed-on: https://chromium-review.googlesource.com/579107 > Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org> > Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> > Cr-Commit-Position: refs/heads/master@{#488599} TBR=yoshiki@chromium.org,yhanada@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: b/62771497, 717455, 747217 Change-Id: I2a4cbf639c1a13c910a1424589074d739fbc259b Reviewed-on: https://chromium-review.googlesource.com/584215 Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org> Cr-Commit-Position: refs/heads/master@{#489271} [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_content_view.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_content_view.h [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_content_view_delegate.h [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_content_view_unittest.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_view.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_view.h [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_view_unittest.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/message_center/views/notification_control_buttons_view.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/message_center/views/notification_control_buttons_view.h
,
Jul 28 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 28 2017
Let me remove the approval. I'll change the approach to refactoring. The crash itself is fixed by the other CL.
,
Aug 1 2017
,
Mar 15 2018
We may need to redesign entire ARC notification view/widget hierarchy, so that let's consider this at that time. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Jul 21 2017