New issue
Advanced search Search tips

Issue 670789 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 670798



Sign in to add a comment

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

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

Issue description

Instead of ash connecting to chrome 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.

 
Blocking: 670798
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 6 2016

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

commit 7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b
Author: jamescook <jamescook@chromium.org>
Date: Tue Dec 06 22:54:18 2016

mash: Have chrome set itself as a controller interface for changing volume

This eliminates a case where ash was directly connecting out to Chrome.

* Add ash::mojom::AcceleratorController interface and use to set the
ash::mojom::VolumeController
* Rename volume controller files to match class names. chrome/browser/ui/ash
is chromeos-only these days so the _chromeos filenames aren't needed.
* Add ash_util::GetAshServiceName() to return "ash" vs. "content_browser"
based on mash vs. classic ash

BUG= 670789 
TEST=ash_unittests, browser_tests, manually change volume with F9/F10
TBR=xiyuan@chromium.org

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

[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/ash/common/accelerators/accelerator_controller.cc
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/ash/common/accelerators/accelerator_controller.h
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/ash/common/mojo_interface_factory.cc
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/ash/mus/manifest.json
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/ash/public/interfaces/BUILD.gn
[add] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/ash/public/interfaces/accelerator_controller.mojom
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/ash/public/interfaces/volume.mojom
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/chromeos/chrome_interface_factory.cc
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/ui/ash/ash_init.cc
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/ui/ash/ash_util.cc
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/ui/ash/ash_util.h
[rename] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/ui/ash/volume_controller.cc
[add] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/ui/ash/volume_controller.h
[rename] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/ui/ash/volume_controller_browsertest.cc
[delete] https://crrev.com/9e11ad5750fb228b83db25fcd90f77488659d750/chrome/browser/ui/ash/volume_controller_chromeos.h
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.h
[modify] https://crrev.com/7e0a04e949287e6ea20e5d2be9fe8a168ac0ae7b/chrome/test/BUILD.gn

Status: Fixed (was: Started)

Comment 5 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 6 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 7 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 8 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 9 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment