Issue 670775 link

Starred by 1 user

Issue metadata

Status:	Archived
Owner:	msw@chromium.org
Closed:	Dec 2016
Cc:	█ mfomitchev@chromium.org jamescook@chromium.org mustash-bugs@google.com
EstimatedDays:	----
NextAction:	----
OS:	Chrome
Pri:	3
Type:	Bug
Proj-Mustash-Mash Proj-Mustash-Chrome VerifyIn-58 VerifyIn-59 VerifyIn-60 VerifyIn-61

Blocking:
issue 670798

Ash shouldn't connect to chrome for ash::mojom::AppListPresenter

Project Member Reported by mfomitchev@chromium.org, Dec 2 2016

Issue description

Instead of ash connecting to chrome for ash::mojom::AppListPresenter, have chrome set the client interface on ash on startup (which it can then use at any point). This enforces the proper layering. 

See https://codereview.chromium.org/2525813004/ for a similar change.

Comment 1 by jamescook@chromium.org, Dec 2 2016

Blocking: 670798

Comment 2 by msw@chromium.org, Dec 8 2016

Cc: mfomitchev@chromium.org
Owner: msw@chromium.org

I can take this; hopefully you haven't started on it yet, Mikhail.

Comment 3 by mfomitchev@chromium.org, Dec 8 2016

Sure, feel free, and thanks!

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 13 2016

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

commit 982c17a70a9c06b191b96e6e77f78933d0989de4
Author: msw <msw@chromium.org>
Date: Tue Dec 13 03:51:22 2016

mash: Have chrome set itself as the app list presenter.

Do not let ash connect to chrome to get the presenter.

Expose mojom::AppList interface in ash to set the presenter.
Chrome's AppListPresenterService connects to register.
Move instance from Chrome's FactoryImpl to AppListServiceAsh.

Implement app_list::AppList; owned by ash::WmShell.

Add export info to app_list's 'presenter' component.
(lets it add the mojom as a public DEPS; thanks Yuzhu!)

Remove connection and interface pointer from AppListPresenterMus.

TODO: What's not TODO here? It's a mess!

BUG= 670775 
TEST=No regressions in chrome --mash app list behavior
R=jamescook@chromium.org,tsepez@chromium.org,xiyuan@chromium.org,rockot@chromium.org

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

[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ash/common/mojo_interface_factory.cc
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ash/common/wm_shell.cc
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ash/common/wm_shell.h
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ash/mus/app_list_presenter_mus.cc
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ash/mus/app_list_presenter_mus.h
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ash/mus/manifest.json
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/chrome/browser/chromeos/chrome_interface_factory.cc
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/chrome/browser/ui/ash/app_list/app_list_presenter_service.cc
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/chrome/browser/ui/ash/app_list/app_list_presenter_service.h
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/chrome/browser/ui/ash/app_list/app_list_service_ash.h
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ui/app_list/DEPS
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ui/app_list/presenter/BUILD.gn
[add] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ui/app_list/presenter/app_list.cc
[add] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ui/app_list/presenter/app_list.h
[modify] https://crrev.com/982c17a70a9c06b191b96e6e77f78933d0989de4/ui/app_list/presenter/app_list_presenter.mojom