New issue
Advanced search Search tips

Issue 756057 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 678705



Sign in to add a comment

mash: Remove ash access from chrome/browser/chromeos/first_run

Project Member Reported by jamescook@chromium.org, Aug 16 2017

Issue description

Replace with mojo apis. See ash/README.md and go/mustash.

 
Blocking: 678705
Components: -Internals>MUS Internals>Services>WindowService
Owner: jamescook@chromium.org
Status: Started (was: Untriaged)
Project Member

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

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

commit f9e0290cfcba215dfe7d1c95f51f216d31546f7d
Author: James Cook <jamescook@chromium.org>
Date: Mon Apr 02 23:24:49 2018

cros: Clean up ash::FirstRunHelper

Clean up before making it work with out-of-process ash (go/mustash).

* Make FirstRunHelper a concrete class
* Eliminate ash::Shell::CreateFirstRunHelper
* Move some methods to FirstRunController in chrome

Bug:  756057 
Test: existing ash_unittests, browser_tests
Change-Id: I6d3660f64e2184a03c2f92b204200a26b5f7dab8
Reviewed-on: https://chromium-review.googlesource.com/990725
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547557}
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/ash/BUILD.gn
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/ash/first_run/first_run_helper.cc
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/ash/first_run/first_run_helper.h
[delete] https://crrev.com/ed5330ab34f3cfbcdf99aeeb104f8e46471f7371/ash/first_run/first_run_helper_impl.cc
[delete] https://crrev.com/ed5330ab34f3cfbcdf99aeeb104f8e46471f7371/ash/first_run/first_run_helper_impl.h
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/ash/first_run/first_run_helper_unittest.cc
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/ash/shell.cc
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/ash/shell.h
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/chromeos_first_run_browsertest.cc
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/first_run_controller.cc
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/first_run_controller.h
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/step.cc
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/step.h
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/steps/app_list_step.cc
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/steps/app_list_step.h
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/steps/help_step.cc
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/steps/help_step.h
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/steps/tray_step.cc
[modify] https://crrev.com/f9e0290cfcba215dfe7d1c95f51f216d31546f7d/chrome/browser/chromeos/first_run/steps/tray_step.h

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 6 2018

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

commit e05cbb9be8be9ca8632b02f693caab4f8dbe4eaa
Author: James Cook <jamescook@chromium.org>
Date: Fri Apr 06 18:58:31 2018

cros: Centralize Widget setup for ash containers

Widget::InitParams setup is different in classic ash vs. mash.
Centralize the boilerplate code. This eliminates some ash/shell.h
includes, making it clearer that there's only one issue here.
It also reduces the chance of weird compiler errors if you
copy/paste the boilerplate but forget the type converters header.

Bug:  756057 
Test: browser_tests
Change-Id: I3f1c76f2b174ef305697f8551a6923534a73dbaf
Reviewed-on: https://chromium-review.googlesource.com/999813
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548875}
[modify] https://crrev.com/e05cbb9be8be9ca8632b02f693caab4f8dbe4eaa/chrome/browser/chromeos/accessibility/chromevox_panel.cc
[modify] https://crrev.com/e05cbb9be8be9ca8632b02f693caab4f8dbe4eaa/chrome/browser/chromeos/lock_screen_apps/toast_dialog_view.cc
[modify] https://crrev.com/e05cbb9be8be9ca8632b02f693caab4f8dbe4eaa/chrome/browser/chromeos/login/ui/gaia_dialog_delegate.cc
[modify] https://crrev.com/e05cbb9be8be9ca8632b02f693caab4f8dbe4eaa/chrome/browser/ui/ash/ash_util.cc
[modify] https://crrev.com/e05cbb9be8be9ca8632b02f693caab4f8dbe4eaa/chrome/browser/ui/ash/ash_util.h
[modify] https://crrev.com/e05cbb9be8be9ca8632b02f693caab4f8dbe4eaa/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/e05cbb9be8be9ca8632b02f693caab4f8dbe4eaa/chrome/browser/ui/views/chrome_web_dialog_view.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 16 2018

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

commit f539884f0d6da21d0c39b82a23d56869da9b97d7
Author: James Cook <jamescook@chromium.org>
Date: Mon Apr 16 20:15:24 2018

cros: Convert some first-run tutorial methods to mojo

This is part of moving ash out-of-process for mustash.

* Introduce FirstRunHelper mojo interface
* Make ash::FirstRunHelper object owned by ash::Shell
* Add explicit create/close widget methods because object is now
  persistent
* Add system tray test API to check if bubble is open

Next step is to move the widget creation into chrome.

Bug:  756057 
Test: ash_unittests, browser_tests
Change-Id: Ie3994bd36b6dfeecd1c685773c5ce8e19394052e
Reviewed-on: https://chromium-review.googlesource.com/998181
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551091}
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/first_run/first_run_helper.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/first_run/first_run_helper.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/first_run/first_run_helper_unittest.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/manifest.json
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/mojo_interface_factory.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/public/interfaces/BUILD.gn
[add] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/public/interfaces/first_run_helper.mojom
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/public/interfaces/system_tray_test_api.mojom
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/shell.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/shell.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/system/tray/system_tray_test_api.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/system/tray/system_tray_test_api.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/chromeos_first_run_browsertest.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/first_run_controller.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/first_run_controller.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/app_list_step.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/app_list_step.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/help_step.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/help_step.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/tray_step.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/tray_step.h

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 16 2018

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

commit 126093fb4c54277c77bc37f23be5a758cc1fb1df
Author: James Cook <jamescook@chromium.org>
Date: Mon Apr 16 22:26:46 2018

cros: Move first-run Widget from ash to chrome

This is another step toward out-of-process ash (go/mustash).

* Create the Widget in FirstRunController in chrome
* Move escape key handling into chrome
* Stop using OverlayEventFilter in ash. The old code wasn't actually
  relying on it for blocking event propagation, just for escape key
  handling and detecting screen lock and shutdown. The old code was
  also watching for LoginStatus changes, but that dates back to the
  use of OverlayEventFilter for partial screenshots (no longer used)
  and isn't relevant to first-run.
* Add mojo interfaces to tell ash to start/stop tutorial

Bug:  756057 
Test: ash_unittests, browser_tests
Change-Id: Icfd88e3541f1393b6a2309120c74bbe6a69b967f
Reviewed-on: https://chromium-review.googlesource.com/1000395
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551147}
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/ash/first_run/first_run_helper.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/ash/first_run/first_run_helper.h
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/ash/first_run/first_run_helper_unittest.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/ash/public/interfaces/first_run_helper.mojom
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/chromeos_first_run_browsertest.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/first_run_controller.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/first_run_controller.h
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/first_run_view.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/first_run_view.h

References in //c/b/chromeos removed, but we still need to fix some access in webui code before it works in mash and the browser_tests can be enabled.

Project Member

Comment 9 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/+/f539884f0d6da21d0c39b82a23d56869da9b97d7

commit f539884f0d6da21d0c39b82a23d56869da9b97d7
Author: James Cook <jamescook@chromium.org>
Date: Mon Apr 16 20:15:24 2018

cros: Convert some first-run tutorial methods to mojo

This is part of moving ash out-of-process for mustash.

* Introduce FirstRunHelper mojo interface
* Make ash::FirstRunHelper object owned by ash::Shell
* Add explicit create/close widget methods because object is now
  persistent
* Add system tray test API to check if bubble is open

Next step is to move the widget creation into chrome.

Bug:  756057 
Test: ash_unittests, browser_tests
Change-Id: Ie3994bd36b6dfeecd1c685773c5ce8e19394052e
Reviewed-on: https://chromium-review.googlesource.com/998181
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551091}
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/first_run/first_run_helper.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/first_run/first_run_helper.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/first_run/first_run_helper_unittest.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/manifest.json
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/mojo_interface_factory.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/public/interfaces/BUILD.gn
[add] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/public/interfaces/first_run_helper.mojom
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/public/interfaces/system_tray_test_api.mojom
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/shell.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/shell.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/system/tray/system_tray_test_api.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/ash/system/tray/system_tray_test_api.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/chromeos_first_run_browsertest.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/first_run_controller.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/first_run_controller.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/app_list_step.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/app_list_step.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/help_step.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/help_step.h
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/tray_step.cc
[modify] https://crrev.com/f539884f0d6da21d0c39b82a23d56869da9b97d7/chrome/browser/chromeos/first_run/steps/tray_step.h

Project Member

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

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

commit 126093fb4c54277c77bc37f23be5a758cc1fb1df
Author: James Cook <jamescook@chromium.org>
Date: Mon Apr 16 22:26:46 2018

cros: Move first-run Widget from ash to chrome

This is another step toward out-of-process ash (go/mustash).

* Create the Widget in FirstRunController in chrome
* Move escape key handling into chrome
* Stop using OverlayEventFilter in ash. The old code wasn't actually
  relying on it for blocking event propagation, just for escape key
  handling and detecting screen lock and shutdown. The old code was
  also watching for LoginStatus changes, but that dates back to the
  use of OverlayEventFilter for partial screenshots (no longer used)
  and isn't relevant to first-run.
* Add mojo interfaces to tell ash to start/stop tutorial

Bug:  756057 
Test: ash_unittests, browser_tests
Change-Id: Icfd88e3541f1393b6a2309120c74bbe6a69b967f
Reviewed-on: https://chromium-review.googlesource.com/1000395
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551147}
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/ash/first_run/first_run_helper.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/ash/first_run/first_run_helper.h
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/ash/first_run/first_run_helper_unittest.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/ash/public/interfaces/first_run_helper.mojom
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/chromeos_first_run_browsertest.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/first_run_controller.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/first_run_controller.h
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/first_run_view.cc
[modify] https://crrev.com/126093fb4c54277c77bc37f23be5a758cc1fb1df/chrome/browser/chromeos/first_run/first_run_view.h

Components: -Internals>Services>WindowService Internals>Services>Ash
Labels: -Proj-Mustash-Mash
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 20 2018

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

commit 9a795ad4744d54ebad122411d6ce5e15a4471861
Author: James Cook <jamescook@chromium.org>
Date: Fri Apr 20 22:39:18 2018

cros: Remove ash::Shell access from first-run webui handler

The old code was reaching into ash to get the shelf alignment. This
won't work with out-of-process ash (see //ash/README.md). Convert to
using ash/public/cpp and profile pref.

This enables the first run browser_tests under mash.

Bug: 770866,  756057 
Test: browser_tests --enable-features=Mash
Change-Id: I43b202ef255301f637593e24a588a42bacdc7fff
Reviewed-on: https://chromium-review.googlesource.com/1022523
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552499}
[delete] https://crrev.com/d4f22170f747a4b2edabf0d41ce69a8482754b89/chrome/browser/ui/webui/chromeos/first_run/DEPS
[modify] https://crrev.com/9a795ad4744d54ebad122411d6ce5e15a4471861/chrome/browser/ui/webui/chromeos/first_run/first_run_ui.cc
[modify] https://crrev.com/9a795ad4744d54ebad122411d6ce5e15a4471861/testing/buildbot/filters/mash.browser_tests.filter

Status: Fixed (was: Started)

Sign in to add a comment