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

Issue 768439 link

Starred by 2 users
Status: Assigned
Owner:
xiy...@chromium.org
Cc:
sadrul@chromium.org
yusukes@chromium.org
elijahtaylor@chromium.org
█ reve...@chromium.org
█ yoshiki@chromium.org
yawano@chromium.org
jamescook@chromium.org
sky@chromium.org
est...@chromium.org
varkha@chromium.org
osh...@chromium.org
hidehiko@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 672961
issue 767990


Sign in to add a comment

Move ArcNotificationSurfaceManager out of the browser process for mus+ash

Project Member Reported by penghuang@chromium.org, Sep 25 2017 
For mus+ash, the exo and ash will not be running in the browser process. The ArcNotificationSurfaceManager in the browser process need access exo surface, ash message center. It will not work. I think we need move ArcNotificationSurfaceManager related code to ash and exo process for mus+ash. And in future, it is better to launch ARC++ apps without the browser process, to support it, we also need move ArcNotificationSurfaceManager and other arc related services to ash process or a dedicated process.

Comment 1 by penghuang@chromium.org, Sep 25 2017

Blockedon: 767990

Comment 2 by penghuang@chromium.org, Sep 25 2017 

For estade@ comment on https://bugs.chromium.org/p/chromium/issues/detail?id=767990#c8

> The plan is for the message center to live in ash on ChromeOS and the 
> browser process on those desktop platforms which don't have native 
> notification integration. The browser process will ask ash to display 
> notifications. All other notifications besides arc only need to pass images
> and text (and respond to inputs); I don't know how arc notifications and exo
> surfaces will be passed across process lines.

Every arc notification is an exo::Surface, and the exo::Surface is backed by an
aura::Window. And the aura::Window will be used to show notification with message
center I think. For cash (Classical ash) and mushrome, arc notification, ash and
exo are running in the browser process, so it just works. For mus+ash, I think
we need move arc notification manager from the browser process to ash and exo process.

Comment 3 by penghuang@chromium.org, Sep 25 2017

Blockedon: 672961
Labels: -Pri-3 Pri-2

Comment 4 by hidehiko@chromium.org, Sep 25 2017 

QQ:
is ash process (or a dedicated process) (easily) accessible to Profile, in mus+ash world?
If not, it's like a bigger problem, as many ARC services depend on Profile, which is actually necessary (e.g., fileapi, tts, voice, settings, notification, bluetooth, auth flow, accessibility, policy, etc...)

Comment 5 by jamescook@chromium.org, Sep 25 2017 

No, ash doesn't have access to Profile. Profile lives in the browser process. (Conceptually, ash just does window management and system UI, whereas chrome/browser is "just an app" running on ash.) Ash has the concept of users, with account ids and such. It can also use prefs, via the mojo pref service.

I suspect ARC services will need to be split into those that really need browser stuff (fileapi, voice, policy) and those that are more system-ui-ish (bluetooth, notifications).

Comment 6 by xiy...@chromium.org, Jan 12 2018

Owner: xiy...@chromium.org
Status: Assigned (was: Untriaged)
Let me think about this...
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 4 2018 

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

commit 4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a
Author: Evan Stade <estade@chromium.org>
Date: Wed Apr 04 17:32:40 2018

Remove two methods from ArcNotificationItem.

- GetAccessibleName()
- IsOpeningSettingsSupported()

These two methods represent fields that are already present in
message_center::Notification.

This also gets rid of ArcNotificationContentViewDelegate in favor
of having the ArcNotificationView create and maintain a reference
to the ArcNotificationContentView directly.


Change-Id: Ifcad792f38902b567f71665046c0d9ba02d74305
Bug: 768439
Reviewed-on: https://chromium-review.googlesource.com/969541
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548114}
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_unittest.cc
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_content_view.cc
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_content_view.h
[delete] https://crrev.com/eb9041c1b5d81033d36aa43344a2805fbb7aa27a/ui/arc/notification/arc_notification_content_view_delegate.h
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_content_view_unittest.cc
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_delegate.cc
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_item.h
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_item_impl.cc
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_item_impl.h
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_view.cc
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_view.h
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/arc_notification_view_unittest.cc
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/mock_arc_notification_item.cc
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/arc/notification/mock_arc_notification_item.h
[modify] https://crrev.com/4f1d8fafd6ca88f6791b3d1838afd75ebdd29b1a/ui/message_center/views/notification_view_md_unittest.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 13 2018 

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

commit 7eb54c85f7c8d6816dfac8c828f419dda3d57ccb
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Fri Apr 13 16:44:56 2018

arc: Move MojoChannel into its own file

Move MojoChannel code out of ArcBridgeHostImpl into its own file
to be shared.

Bug: 768439
Change-Id: I730ae1f498893455f3aa2e768f47893409109768
Reviewed-on: https://chromium-review.googlesource.com/1012136
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550646}
[modify] https://crrev.com/7eb54c85f7c8d6816dfac8c828f419dda3d57ccb/components/arc/BUILD.gn
[modify] https://crrev.com/7eb54c85f7c8d6816dfac8c828f419dda3d57ccb/components/arc/arc_bridge_host_impl.cc
[modify] https://crrev.com/7eb54c85f7c8d6816dfac8c828f419dda3d57ccb/components/arc/arc_bridge_host_impl.h
[add] https://crrev.com/7eb54c85f7c8d6816dfac8c828f419dda3d57ccb/components/arc/mojo_channel.h
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7eb54c85f7c8d6816dfac8c828f419dda3d57ccb

commit 7eb54c85f7c8d6816dfac8c828f419dda3d57ccb
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Fri Apr 13 16:44:56 2018

arc: Move MojoChannel into its own file

Move MojoChannel code out of ArcBridgeHostImpl into its own file
to be shared.

Bug: 768439
Change-Id: I730ae1f498893455f3aa2e768f47893409109768
Reviewed-on: https://chromium-review.googlesource.com/1012136
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550646}
[modify] https://crrev.com/7eb54c85f7c8d6816dfac8c828f419dda3d57ccb/components/arc/BUILD.gn
[modify] https://crrev.com/7eb54c85f7c8d6816dfac8c828f419dda3d57ccb/components/arc/arc_bridge_host_impl.cc
[modify] https://crrev.com/7eb54c85f7c8d6816dfac8c828f419dda3d57ccb/components/arc/arc_bridge_host_impl.h
[add] https://crrev.com/7eb54c85f7c8d6816dfac8c828f419dda3d57ccb/components/arc/mojo_channel.h
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 25 2018 

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

commit 3619737e6625711c7628823aaee8ce2f9543f744
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Wed Apr 25 00:45:17 2018

Handle ARC notifications in ash

Makes ui/arc code run inside ash instead of the browser. The code
itself will be moved to //ash in follow-up CLs.

- ArcNotificationManager lives in ash::MessageCenterController;
- Arc bridge host forwards the NotificationsInstance mojo interface
  to ash via AshMessageCenterController interface;
- ArcNotificationSurfaceManager lives in ash::WaylandServerController;

Bug: 768439
Change-Id: Id3572a64cd4d8fac709cd2c22f86b392571d5041
Reviewed-on: https://chromium-review.googlesource.com/1011205
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553397}
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/BUILD.gn
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/DEPS
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/message_center/message_center_controller.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/message_center/message_center_controller.h
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/public/interfaces/BUILD.gn
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/public/interfaces/ash_message_center_controller.mojom
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/shell.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/shell.h
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/shell/content/client/shell_browser_main_parts.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/wayland/BUILD.gn
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/wayland/wayland_server_controller.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/wayland/wayland_server_controller.h
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ash/window_manager.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/chrome/browser/chromeos/arc/arc_service_launcher.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/chrome/browser/exo_parts.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/chrome/browser/exo_parts.h
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/components/arc/BUILD.gn
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/components/arc/DEPS
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/components/arc/arc_bridge_host_impl.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/components/arc/arc_bridge_service.h
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/components/arc/common/BUILD.gn
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ui/arc/BUILD.gn
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ui/arc/notification/arc_notification_content_view.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ui/arc/notification/arc_notification_item_impl.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ui/arc/notification/arc_notification_manager.cc
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ui/arc/notification/arc_notification_manager.h
[modify] https://crrev.com/3619737e6625711c7628823aaee8ce2f9543f744/ui/arc/notification/arc_notification_manager_unittest.cc

Comment 12 by est...@chromium.org, Apr 25 2018

Cc: yawano@chromium.org
ArcAccessibilityHelperBridge still reaches into //ui/arc/notifications --- what's the plan for that?

related, I see a lot of ArcAccessibilityHelperBridge code commented with:

  // TODO(yawano): Remove this after this becomes unnecessary.

but it's not clear when that is.
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 25 2018 

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

commit 159ac40e3d9c876a958a180f0a6c7173e8eb3ff3
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Wed Apr 25 17:08:07 2018

Use AshMessageCenterClient to get arc app id

Restores the arc app notification badging on shelf icons by re-wiring
ArcNotificationManager get app id code path through mojo.

- Change ArcNotificationManager to handle get app id asynchronously;
- Add GetArcAppIdByPackageName to AshMessageCenterClient mojo interface;
- ArcNotificationManager use the mojo call to get app id;

Bug: 768439
Change-Id: I199636b374d539677d93e35be1fd4f7b9b900bca
Reviewed-on: https://chromium-review.googlesource.com/1012123
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553603}
[modify] https://crrev.com/159ac40e3d9c876a958a180f0a6c7173e8eb3ff3/ash/message_center/message_center_controller.cc
[modify] https://crrev.com/159ac40e3d9c876a958a180f0a6c7173e8eb3ff3/ash/message_center/message_center_controller.h
[modify] https://crrev.com/159ac40e3d9c876a958a180f0a6c7173e8eb3ff3/ash/message_center/notifier_settings_view_unittest.cc
[modify] https://crrev.com/159ac40e3d9c876a958a180f0a6c7173e8eb3ff3/ash/public/interfaces/ash_message_center_controller.mojom
[modify] https://crrev.com/159ac40e3d9c876a958a180f0a6c7173e8eb3ff3/chrome/browser/notifications/chrome_ash_message_center_client.cc
[modify] https://crrev.com/159ac40e3d9c876a958a180f0a6c7173e8eb3ff3/chrome/browser/notifications/chrome_ash_message_center_client.h
[modify] https://crrev.com/159ac40e3d9c876a958a180f0a6c7173e8eb3ff3/ui/arc/notification/arc_notification_manager.cc
[modify] https://crrev.com/159ac40e3d9c876a958a180f0a6c7173e8eb3ff3/ui/arc/notification/arc_notification_manager.h

Comment 14 by xiy...@chromium.org, Apr 25 2018 

Re #12: Good findings. It seems ArcAccessibilityHelperBridge only needs the notification surface for two calls: NotifyAccessibilityEvent [1] and SetAxTreeId [2]. We need to replace them with mojo. The question is in which interface, not sure whether ash.mojom.AshMessageCenterController is a good fit.

[1]: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc?rcl=2597d16f9b34502184fbff20d5ebe3203d383061&l=302
[2]: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc?rcl=2597d16f9b34502184fbff20d5ebe3203d383061&l=390

Comment 15 by yawano@chromium.org, Apr 26 2018 

For comment 12, it's about this mojom interface change, https://crrev.com/c/844052.

We had to implement Chrome side in a backward compatible way. We can remove those logics once there would be no instance with old version of interface.

Comment 16 by estade@google.com, Apr 26 2018 

Instance of arc? when will that be? Can we add detail (e.g. bug links) to the comments so developers know when this can be cleaned up? It will probably be easier to update the code for mash after that, if it's going to happen any time soon.

Comment 17 by yawano@chromium.org, Apr 27 2018 

Yes, it's an instance of arc. I think we can remove it now as the interface change has happened on M66. I'll remove those dead code in this or next week and update this bug after it's removed.

Comment 18 by estade@google.com, Apr 27 2018 

Great, thanks!

Comment 19 by yawano@chromium.org, May 2 2018 

CL to remove backward compat code is in review at https://crrev.com/c/1039405.
Project Member

Comment 20 by bugdroid1@chromium.org, May 8 2018 

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

commit 522710a2c70bb34e1b85ff73a07ea39a8c81b43e
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Tue May 08 16:35:42 2018

ash: system/web_notification -> message_center

Rename ash/system/web_notification to message_center/ since it holds
ash message center UI code rather than web notification related code.

Also fixed a few presubmit warnings.

Bug: 768439
Change-Id: Ifaf7b8795ebdebff482dc9ac25ab9961dcb6d939
Reviewed-on: https://chromium-review.googlesource.com/1047986
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556829}
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/BUILD.gn
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/accelerators/accelerator_controller.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/accessibility/key_accessibility_enabler_unittest.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/message_center/message_center_controller.h
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/session/session_controller_unittest.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/shelf/shelf_layout_manager.h
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/shell.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/shell.h
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/shell/window_type_launcher.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/sidebar/sidebar_widget.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/caps_lock_notification_controller_unittest.cc
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/ash_popup_alignment_delegate.cc
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/ash_popup_alignment_delegate.h
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/ash_popup_alignment_delegate_unittest.cc
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/fullscreen_notification_blocker.cc
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/fullscreen_notification_blocker.h
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/inactive_user_notification_blocker.cc
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/inactive_user_notification_blocker.h
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/inactive_user_notification_blocker_unittest.cc
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/notification_tray.cc
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/notification_tray.h
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/notification_tray_unittest.cc
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/session_state_notification_blocker.cc
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/session_state_notification_blocker.h
[rename] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/message_center/session_state_notification_blocker_unittest.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/screen_layout_observer_unittest.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/status_area_widget.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/status_area_widget.h
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/status_area_widget_unittest.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/tray/system_tray.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/tray/system_tray.h
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/tray/system_tray_unittest.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/tray_caps_lock_unittest.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/tray_drag_controller.h
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/system/unified/unified_system_tray.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ash/wm/panels/panel_layout_manager_unittest.cc
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/chrome/browser/notifications/message_center_notification_manager.h
[modify] https://crrev.com/522710a2c70bb34e1b85ff73a07ea39a8c81b43e/ui/arc/notification/arc_notification_content_view_unittest.cc
Project Member

Comment 21 by bugdroid1@chromium.org, May 11 2018 

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

commit 4139f887459424ef31d1b5d005e7bab73a1014b7
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Fri May 11 15:12:25 2018

ash: More WebNotification -> Notification renaming

Missed from previous CL:
  https://chromium-review.googlesource.com/1047986

Bug: 768439
Change-Id: Icc7cdcdd5084b9203abeec34e0244c43312ff4b6
Reviewed-on: https://chromium-review.googlesource.com/1054173
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557883}
[modify] https://crrev.com/4139f887459424ef31d1b5d005e7bab73a1014b7/ash/system/message_center/ash_popup_alignment_delegate.h
[modify] https://crrev.com/4139f887459424ef31d1b5d005e7bab73a1014b7/ash/system/message_center/notification_tray.cc
[modify] https://crrev.com/4139f887459424ef31d1b5d005e7bab73a1014b7/ash/system/message_center/notification_tray.h
[modify] https://crrev.com/4139f887459424ef31d1b5d005e7bab73a1014b7/ash/system/message_center/notification_tray_unittest.cc
[modify] https://crrev.com/4139f887459424ef31d1b5d005e7bab73a1014b7/ash/system/status_area_widget_unittest.cc
[modify] https://crrev.com/4139f887459424ef31d1b5d005e7bab73a1014b7/ash/system/tray/system_tray.cc
[modify] https://crrev.com/4139f887459424ef31d1b5d005e7bab73a1014b7/ash/system/tray/system_tray.h
Project Member

Comment 22 by bugdroid1@chromium.org, May 15 2018 

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

commit a0a832601d07383345e4cbbb8048d5f829f64c17
Author: Yuki Awano <yawano@chromium.org>
Date: Tue May 15 04:20:06 2018

Remove backward compat code from ArcAccessibilityHelperBridge

- Interface change has happend at https://crrev.com/c/844052 around the
  end of February. It should be fine to remove backward compat code now.

Bug: 768439
Test: unit_tests:ArcAccessibilityHelperBridgeTest
Change-Id: Ibac321f3889d605393928839b5292e64d27b0094
Reviewed-on: https://chromium-review.googlesource.com/1039405
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: Yuki Awano <yawano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558605}
[modify] https://crrev.com/a0a832601d07383345e4cbbb8048d5f829f64c17/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc
[modify] https://crrev.com/a0a832601d07383345e4cbbb8048d5f829f64c17/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h
[modify] https://crrev.com/a0a832601d07383345e4cbbb8048d5f829f64c17/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_unittest.cc
Project Member

Comment 23 by bugdroid1@chromium.org, May 16 2018 

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

commit 055e441845d88aaa776d2bc47cef89704499be71
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Wed May 16 23:50:11 2018

Move ui/arc into ash/system/message_center/arc

- Move ui/arc into ash/system/message_center/arc
- Merge ui_arc_unittests into ash_unittests

Bug: 768439
Change-Id: I56dd855ee2b4defefd9c57fd0c1b5bda453e61e7
Reviewed-on: https://chromium-review.googlesource.com/1053302
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559339}
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/BUILD.gn
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/BUILD.gn
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/message_center/message_center_controller.cc
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/message_center/message_center_controller.h
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/message_center/message_center_view.h
[add] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/BUILD.gn
[add] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/DEPS
[add] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/OWNERS
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_content_view.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_content_view.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_content_view_unittest.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_delegate.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_delegate.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_item.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_item_impl.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_item_impl.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_manager.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_manager.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_manager_unittest.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_surface.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_surface_impl.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_surface_impl.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_surface_manager.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_surface_manager.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_surface_manager_impl.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_surface_manager_impl.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_view.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_view.h
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/arc_notification_view_unittest.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/mock_arc_notification_item.cc
[rename] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/system/message_center/arc/mock_arc_notification_item.h
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/wayland/BUILD.gn
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/wayland/wayland_server_controller.cc
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/ash/wayland/wayland_server_controller.h
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_unittest.cc
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/components/arc/BUILD.gn
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/testing/buildbot/chromium.chromiumos.json
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/testing/buildbot/chromium.memory.json
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/testing/buildbot/gn_isolate_map.pyl
[modify] https://crrev.com/055e441845d88aaa776d2bc47cef89704499be71/testing/buildbot/test_suites.pyl
[delete] https://crrev.com/9eb5889a41b52904bf01d2b57f8e519403a27ae0/ui/arc/BUILD.gn
[delete] https://crrev.com/9eb5889a41b52904bf01d2b57f8e519403a27ae0/ui/arc/DEPS
[delete] https://crrev.com/9eb5889a41b52904bf01d2b57f8e519403a27ae0/ui/arc/OWNERS
[delete] https://crrev.com/9eb5889a41b52904bf01d2b57f8e519403a27ae0/ui/arc/notification/DEPS
[delete] https://crrev.com/9eb5889a41b52904bf01d2b57f8e519403a27ae0/ui/arc/notification/OWNERS
[delete] https://crrev.com/9eb5889a41b52904bf01d2b57f8e519403a27ae0/ui/arc/test/run_all_unittests.cc
Project Member

Comment 24 by bugdroid1@chromium.org, May 18 2018 

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

commit 474c66c13b75be3bca80e0249a157d42c0c70ba1
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Fri May 18 05:11:22 2018

ash: Move kDefaultArcNotifierId to a proper header

Move kDefaultArcNotifierId from shelf_constants.h to
arc_notification_constants.h and eliminate one nogncheck.

Bug: 768439
Change-Id: Ib2837e410b9c1fbb9d48a4f6ca15694e2ca39017
Reviewed-on: https://chromium-review.googlesource.com/1065128
Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559803}
[modify] https://crrev.com/474c66c13b75be3bca80e0249a157d42c0c70ba1/ash/shelf/shelf_constants.h
[modify] https://crrev.com/474c66c13b75be3bca80e0249a157d42c0c70ba1/ash/shelf/shelf_controller.cc
[modify] https://crrev.com/474c66c13b75be3bca80e0249a157d42c0c70ba1/ash/system/message_center/arc/BUILD.gn
[add] https://crrev.com/474c66c13b75be3bca80e0249a157d42c0c70ba1/ash/system/message_center/arc/arc_notification_constants.h
[modify] https://crrev.com/474c66c13b75be3bca80e0249a157d42c0c70ba1/ash/system/message_center/arc/arc_notification_item_impl.cc
Project Member

Comment 25 by bugdroid1@chromium.org, May 18 2018 

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

commit 8e16a11d919e8558943b50fbf5500526d275577e
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Fri May 18 13:32:53 2018

ash: Delegate interface for ArcNotificationManager

Add an ArcNotificationManagerDelegate interface for arc notification
code to call ash code without the need to add "nogncheck".

Bug: 768439
Change-Id: Id0d992539af90cd758a15cdba58aafb73212fb52
Reviewed-on: https://chromium-review.googlesource.com/1065121
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559876}
[modify] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/BUILD.gn
[add] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/message_center/arc_notification_manager_delegate_impl.cc
[add] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/message_center/arc_notification_manager_delegate_impl.h
[modify] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/message_center/message_center_controller.cc
[modify] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/message_center/message_center_controller.h
[modify] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/system/message_center/arc/BUILD.gn
[modify] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/system/message_center/arc/arc_notification_manager.cc
[modify] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/system/message_center/arc/arc_notification_manager.h
[add] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/system/message_center/arc/arc_notification_manager_delegate.h
[modify] https://crrev.com/8e16a11d919e8558943b50fbf5500526d275577e/ash/system/message_center/arc/arc_notification_manager_unittest.cc

Comment 26 by xiy...@chromium.org, May 21 2018 

For record, remaining work items:
1. Get rid of "ash/wm/window_util.h" include in ArcNotificationContentView
   for ash::wm::SnapWindowToPixelBoundary. Maybe through
   ArcNotificationManagerDelegate interface, or move SnapWindowToPixelBoundary
   somewhere;

2. ArcAccessibilityHelperBridge accesses ArcNotificationSurface as #14
   and should use an alternative way instead of directly accessing it;

Comment 27 by sky@chromium.org, Aug 15

Labels: -Proj-Mustash Proj-Mash-SingleProcess
Xiyuan, do we need this for single-process mash, or only multi-process? I'm assuming we need it for single-process as well. If that isn't the case, please update the label.

Comment 28 by xiy...@chromium.org, Aug 15

Labels: -Proj-Mash-SingleProcess Proj-Mash-MultiProcess
The biggest remaining issue is bullet 2 in #26, where ArcAccessibilityHelperBridge accesses ArcNotificationSurface directly. This should work in single process mash. But not mult-process.

Comment 29 by lhchavez@chromium.org, Jan 7

Cc: -lhchavez@chromium.org
►

Sign in to add a comment