New issue
Advanced search Search tips

Issue 826415 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 678705



Sign in to add a comment

mash: Remove ash shelf_model access from chrome's AppInfoFooterPanel

Project Member Reported by jamescook@chromium.org, Mar 27 2018

Issue description

See //c/b/ui/views/apps/app_info_dialog/app_info_footer_panel.cc

It pokes into ash::Shell::Get()->shelf_model() to figure out if an app is pinned. Maybe it should use the mirrored copy in ChromeLauncherController.


 

Comment 1 by msw@chromium.org, Mar 27 2018

Owner: msw@chromium.org
Status: Assigned (was: Untriaged)
Yes, if it lives in Chrome, it should use the mirrored ShelfModel.
It already uses chrome_launcher_controller_util.h, so this should be easy.
(mash's "app info" dialogs shown via the app list are missing the pin button)

I can hang onto this bug myself for now, but may not get to it right away.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 31 2018

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

commit 21c40a5db3961422c6ee7c9520f4484120aac93e
Author: Mike Wasserman <msw@chromium.org>
Date: Sat Mar 31 01:48:53 2018

mash: Support shelf pin and unpin in app info dialogs.

Use ChromeLauncherController's ShelfModel to get/set shelf pinning.
Ash's Shell is inaccessible from Chrome in Mash.

Fix unit tests to construct a ChromeLauncherController and ShelfModel.
Make BrowserStatusMonitor check for a Shell to support non-Mash tests.
Convert AshTestBase fixture to BrowserWithTestWindowTest; disable sync.

Bug:  826415 
Test: App list -> app context menu -> app info -> dialog shows [un]pin.
Change-Id: Icdfdde84692184cf95dee83a71ff068cef5f98e9
Reviewed-on: https://chromium-review.googlesource.com/988825
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547356}
[modify] https://crrev.com/21c40a5db3961422c6ee7c9520f4484120aac93e/chrome/browser/ui/ash/launcher/browser_status_monitor.cc
[delete] https://crrev.com/63c57a3be49c97989ab0f644186e8dc05eddabc7/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_ash_unittest.cc
[modify] https://crrev.com/21c40a5db3961422c6ee7c9520f4484120aac93e/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views.h
[modify] https://crrev.com/21c40a5db3961422c6ee7c9520f4484120aac93e/chrome/browser/ui/views/apps/app_info_dialog/app_info_dialog_views_unittest.cc
[modify] https://crrev.com/21c40a5db3961422c6ee7c9520f4484120aac93e/chrome/browser/ui/views/apps/app_info_dialog/app_info_footer_panel.cc
[modify] https://crrev.com/21c40a5db3961422c6ee7c9520f4484120aac93e/chrome/browser/ui/views/apps/app_info_dialog/app_info_footer_panel.h
[modify] https://crrev.com/21c40a5db3961422c6ee7c9520f4484120aac93e/chrome/test/BUILD.gn

Comment 3 by msw@chromium.org, Mar 31 2018

Status: Fixed (was: Assigned)

Sign in to add a comment