New issue
Advanced search Search tips

Issue 681072 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug


Sign in to add a comment

Shelf works well for 30-60 minutes of casual browsing

Project Member Reported by sky@chromium.org, Jan 13 2017

Issue description

This should include things like clicking to activate/restore, pinning, context menu... Depending upon the amount of work needed it may make sense to file specific bugs and mark them as blockers for this.
 

Comment 1 by msw@chromium.org, Jan 13 2017

Owner: msw@chromium.org
Status: Started (was: Untriaged)
I'm actively working on this; most of my changes have pointed at  Issue 557406 .
I'll scan through to find some existing blocking bugs, and file more.
There's still a substantial amount of work to do here.

Comment 2 by msw@chromium.org, Jan 13 2017

Blockedon: 679087 681119 672599 631216 640693 647879
I've found some open blockers; perhaps some aren't critical (panels, auto-hide)?
There are *many* more missing features; basically everything in c/b/ui/ash/launcher.

I may need to skip some cleanup and rethink my strategy to speed up my progress here. Instead of shifting chrome-side item delegate and controller responsibilities to ash's ShelfWindowWatcher and ShelfController, maybe I should try to use more of the existing ash<->chrome delegation by making shelf.mojom better match the existing patterns (of ChromeLauncherController, LauncherItemController, etc.). I'll continue thinking along these lines and draft a plan.
Autohide matters more to me than context menu (or even having pinned apps at all).

Comment 4 by msw@chromium.org, Feb 14 2017

Components: UI>Shell>Shelf

Comment 5 by msw@chromium.org, Feb 14 2017

Blockedon: 692141

Comment 6 by msw@chromium.org, Jun 8 2017

Blockedon: 730872

Comment 7 by msw@chromium.org, Jun 8 2017

Blockedon: 730876

Comment 8 by msw@chromium.org, Jun 8 2017

Blockedon: 730884

Comment 9 by msw@chromium.org, Jun 8 2017

Blockedon: 730886

Comment 10 by msw@chromium.org, Jun 8 2017

Blockedon: 730887

Comment 11 by msw@chromium.org, Jun 8 2017

Blockedon: 730890

Comment 12 by msw@chromium.org, Aug 10 2017

Blockedon: -730872

Comment 13 by msw@chromium.org, Aug 17 2017

Blockedon: 756286

Comment 14 by msw@chromium.org, Aug 24 2017

Blockedon: -730886

Comment 15 by msw@chromium.org, Sep 12 2017

Blockedon: 764394
Project Member

Comment 16 by bugdroid1@chromium.org, Sep 15 2017

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

commit 12d3ac7d82d48aeef4411f929e41b1a1fc1216ac
Author: Mike Wasserman <msw@chromium.org>
Date: Fri Sep 15 21:15:10 2017

cros: Unify cash and mash ChromeLauncherController management.

Refine ChromeLauncherController lifetime management.
(always use the new ChromeLauncherControllerInitializer)

Remove ShellDelegate::ShelfInit and ShelfShutdown.
Remove ShelfInitializer (tests/shell init via default prefs).
Cleanup (std::make_unique, TestShellDelegate usage, etc)
Reorder display update test expectations (unclear what changed...)

Bug:  557406 ,  681072 
Change-Id: I5cd35b711c1d96761ac2204f7ba738eef58829ab
Reviewed-on: https://chromium-review.googlesource.com/667838
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502376}
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/ash/display/display_manager_unittest.cc
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/ash/mus/shell_delegate_mus.h
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/ash/shell.cc
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/ash/shell_delegate.h
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/ash/test_shell_delegate.cc
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/ash/test_shell_delegate.h
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/chrome/browser/ui/ash/chrome_shell_delegate.h
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
[modify] https://crrev.com/12d3ac7d82d48aeef4411f929e41b1a1fc1216ac/chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Sep 18 2017

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

commit ac26b921c785cd45429ebd49465eeefbf6b34827
Author: Max Morin <maxmorin@chromium.org>
Date: Mon Sep 18 08:59:13 2017

Revert "cros: Unify cash and mash ChromeLauncherController management."

This reverts commit 12d3ac7d82d48aeef4411f929e41b1a1fc1216ac.

Reason for revert: Some DemoAppLauncherTest.* and ChromeLauncherController.* tests failing on Linux ChromiumOS MSan Tests. https://build.chromium.org/p/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests/builds/2997

Example logs:
[ RUN      ] DemoAppLauncherTest.NoNetwork
[13368:13368:0915/161331.895126:ERROR:input_method_manager_impl.cc(1028)] IMEEngine for "fgoepimhcoialccpbmpnnblemnepkkao" is not registered
[13368:13443:0915/161332.029936:WARNING:freezer_cgroup_process_manager.cc(62)] Cgroup freezer does not exist or is not writable. Unable to freeze renderer processes.
[13368:13368:0915/161332.155113:ERROR:touch_devices_controller.cc(22)] Failed to set touchpad enabled state.
[13368:13368:0915/161334.670608:WARNING:CONSOLE(0)] "Styling master document from stylesheets defined in HTML Imports is deprecated, and is planned to be removed in M65, around March 2018. Please refer to https://goo.gl/EGXzpw for possible migration paths.", source:  (0)
[13368:13368:0915/161419.402418:ERROR:device_event_log_impl.cc(156)] [16:14:19.402] Network: network_state_handler.cc:185 SetTechnologyEnabled() called for the Tether DeviceState, but the current state was: 0
[13368:13368:0915/161419.456610:INFO:CONSOLE(9476)] "Loading asset bundle oauth-enrollment", source: chrome://oobe/oobe.js (9476)
[13368:13368:0915/161419.495392:ERROR:kiosk_profile_loader.cc(113)] Cryptohome is mounted before launching kiosk app.
[13368:13368:0915/161419.683256:ERROR:input_method_manager_impl.cc(1028)] IMEEngine for "fgoepimhcoialccpbmpnnblemnepkkao" is not registered
[13368:13368:0915/161419.786358:ERROR:profile_helper.cc(344)] ProfileHelper::GetProfileByUserUnsafe is called when |user|'s profile is not created. It probably means that something is wrong with a calling code. Please report in http://crbug.com/361528 if you see this message.
[13368:13368:0915/161419.786447:ERROR:profile_helper.cc(344)] ProfileHelper::GetProfileByUserUnsafe is called when |user|'s profile is not created. It probably means that something is wrong with a calling code. Please report in http://crbug.com/361528 if you see this message.
[13368:13368:0915/161419.786529:ERROR:profile_helper.cc(344)] ProfileHelper::GetProfileByUserUnsafe is called when |user|'s profile is not created. It probably means that something is wrong with a calling code. Please report in http://crbug.com/361528 if you see this message.
[13368:13368:0915/161419.791385:WARNING:demo_app_launcher.cc(91)] Disabling network before launching demo app..
[13368:13368:0915/161419.791569:ERROR:device_event_log_impl.cc(156)] [16:14:19.791] Network: network_state_handler.cc:185 SetTechnologyEnabled() called for the Tether DeviceState, but the current state was: 0
[13368:13368:0915/161419.823204:ERROR:profile_helper.cc(344)] ProfileHelper::GetProfileByUserUnsafe is called when |user|'s profile is not created. It probably means that something is wrong with a calling code. Please report in http://crbug.com/361528 if you see this message.
[13368:13368:0915/161419.908166:ERROR:multi_user_window_manager_stub.cc(60)] Not implemented reached in virtual void chrome::MultiUserWindowManagerStub::RemoveObserver(chrome::MultiUserWindowManager::Observer *)
[13368:13368:0915/161420.126551:ERROR:touch_devices_controller.cc(22)] Failed to set touchpad enabled state.
[13368:13368:0915/161421.334333:ERROR:device_event_log_impl.cc(156)] [16:14:21.334] Network: network_state_handler.cc:185 SetTechnologyEnabled() called for the Tether DeviceState, but the current state was: 0
==13368==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x174c40a2 in HasObserver base/observer_list.h:299:3
    #1 0x174c40a2 in aura::Window::HasObserver(aura::WindowObserver const*) const ui/aura/window.cc:488:0
    #2 0x174e2185 in aura::WindowObserver::OnUnobservingWindow(aura::Window*) ui/aura/window_observer.cc:25:15
    #3 0x174b9b34 in aura::Window::RemoveObserver(aura::WindowObserver*) ui/aura/window.cc:483:13
    #4 0x1ca8ce6d in RemoveAll base/scoped_observer.h:45:20
    #5 0x1ca8ce6d in ~ScopedObserver base/scoped_observer.h:26:0
    #6 0x1ca8ce6d in AppWindowLauncherItemController::~AppWindowLauncherItemController() chrome/browser/ui/ash/launcher/app_window_launcher_item_controller.cc:25:0
    #7 0x1c27609c in ~ExtensionAppWindowLauncherItemController chrome/browser/ui/ash/launcher/extension_app_window_launcher_item_controller.cc:23:50
    #8 0x1c27609c in ExtensionAppWindowLauncherItemController::~ExtensionAppWindowLauncherItemController() chrome/browser/ui/ash/launcher/extension_app_window_launcher_item_controller.cc:23:0
    #9 0x1b242b8e in operator() buildtools/third_party/libc++/trunk/include/memory:2272:5
    #10 0x1b242b8e in reset buildtools/third_party/libc++/trunk/include/memory:2585:0
    #11 0x1b242b8e in ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2539:0
    #12 0x1b242b8e in ~pair buildtools/third_party/libc++/trunk/include/utility:312:0
    #13 0x1b242b8e in __destroy<std::__1::pair<const ash::ShelfID, std::__1::unique_ptr<ash::ShelfItemDelegate, std::__1::default_delete<ash::ShelfItemDelegate> > > > buildtools/third_party/libc++/trunk/include/memory:1726:0
    #14 0x1b242b8e in destroy<std::__1::pair<const ash::ShelfID, std::__1::unique_ptr<ash::ShelfItemDelegate, std::__1::default_delete<ash::ShelfItemDelegate> > > > buildtools/third_party/libc++/trunk/include/memory:1589:0
    #15 0x1b242b8e in std::__1::__tree<std::__1::__value_type<ash::ShelfID, std::__1::unique_ptr<ash::ShelfItemDelegate, std::__1::default_delete<ash::ShelfItemDelegate> > >, std::__1::__map_value_compare<ash::ShelfID, std::__1::__value_type<ash::ShelfID, std::__1::unique_ptr<ash::ShelfItemDelegate, std::__1::default_delete<ash::ShelfItemDelegate> > >, std::__1::less<ash::ShelfID>, true>, std::__1::allocator<std::__1::__value_type<ash::ShelfID, std::__1::unique_ptr<ash::ShelfItemDelegate, std::__1::default_delete<ash::ShelfItemDelegate> > > > >::destroy(std::__1::__tree_node<std::__1::__value_type<ash::ShelfID, std::__1::unique_ptr<ash::ShelfItemDelegate, std::__1::default_delete<ash::ShelfItemDelegate> > >, void*>*) buildtools/third_party/libc++/trunk/include/__tree:1831:0
    #16 0x1b23d0cc in clear buildtools/third_party/libc++/trunk/include/__tree:1868:5
    #17 0x1b23d0cc in clear buildtools/third_party/libc++/trunk/include/map:1202:0
    #18 0x1b23d0cc in ash::ShelfModel::DestroyItemDelegates() ash/public/cpp/shelf_model.cc:118:0
    #19 0x1b405026 in ash::Shell::~Shell() ash/shell.cc:787:18
    #20 0x1b40bacc in ash::Shell::~Shell() ash/shell.cc:645:17
    #21 0x1ca7ba24 in AshInit::~AshInit() chrome/browser/ui/ash/ash_init.cc:143:5
    #22 0x1c2ba774 in operator() buildtools/third_party/libc++/trunk/include/memory:2272:5
    #23 0x1c2ba774 in reset buildtools/third_party/libc++/trunk/include/memory:2585:0
    #24 0x1c2ba774 in ChromeBrowserMainExtraPartsAsh::PostMainMessageLoopRun() chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc:167:0
    #25 0xf69ca8d in ChromeBrowserMainParts::PostMainMessageLoopRun() chrome/browser/chrome_browser_main.cc:1948:29
    #26 0x6047fc8 in chromeos::ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() chrome/browser/chromeos/chrome_browser_main_chromeos.cc:1132:32
    #27 0x8e976d2 in content::BrowserMainLoop::ShutdownThreadsAndCleanUp() content/browser/browser_main_loop.cc:1221:13
    #28 0x8ea2b09 in content::BrowserMainRunnerImpl::Shutdown() content/browser/browser_main_runner.cc:208:19
    #29 0x8e812fa in content::BrowserMain(content::MainFunctionParams const&) content/browser/browser_main.cc:48:16
    #30 0xef181a0 in content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:425:14
    #31 0xef1b0bf in content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:703:12
    #32 0x17b48a92 in service_manager::Main(service_manager::MainParams const&) services/service_manager/embedder/main.cc:469:29
    #33 0xef14897 in content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:10
    #34 0x10cc6dd9 in content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:272:3
    #35 0xf55dd6e in InProcessBrowserTest::SetUp() chrome/test/base/in_process_browser_test.cc:267:20
    #36 0x4db800a in chromeos::DemoAppLauncherTest::SetUp() chrome/browser/chromeos/login/demo_mode/demo_app_launcher_browsertest.cc:82:27
    #37 0x6d99a6b in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc:2456:12
    #38 0x6d99a6b in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2468:0
    #39 0x6d9d617 in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2654:11
    #40 0x6d9eff9 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2772:28
    #41 0x6dc00b4 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4677:43
    #42 0x6dbef29 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> third_party/googletest/src/googletest/src/gtest.cc:2456:12
    #43 0x6dbef29 in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4285:0
    #44 0xf5cbe63 in RUN_ALL_TESTS third_party/googletest/src/googletest/include/gtest/gtest.h:2237:46
    #45 0xf5cbe63 in base::TestSuite::Run() base/test/test_suite.cc:270:0
    #46 0xf08e61d in ChromeTestSuiteRunner::RunTestSuite(int, char**) chrome/test/base/chrome_test_launcher.cc:70:38
    #47 0x10e110fe in content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:520:31
    #48 0xf08f6f3 in LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) chrome/test/base/chrome_test_launcher.cc:160:10
    #49 0xf0837d1 in main chrome/test/base/browser_tests_main_chromeos.cc:26:10
    #50 0x7f354fa64f44 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287:0
    #51 0x915d0f in _start ??:0:0
  Uninitialized value was created by a heap deallocation
    #0 0x983709 in operator delete(void*) ??:0:0
    #1 0x1b36ccdc in ash::RootWindowController::CloseChildWindows() ash/root_window_controller.cc:536:7
    #2 0x1b40b74a in ash::Shell::CloseAllRootWindowChildWindows() ash/shell.cc:1183:19
    #3 0x1b404248 in ash::Shell::~Shell() ash/shell.cc:732:3
    #4 0x1b40bacc in ash::Shell::~Shell() ash/shell.cc:645:17
    #5 0x1ca7ba24 in AshInit::~AshInit() chrome/browser/ui/ash/ash_init.cc:143:5
    #6 0x1c2ba774 in operator() buildtools/third_party/libc++/trunk/include/memory:2272:5
    #7 0x1c2ba774 in reset buildtools/third_party/libc++/trunk/include/memory:2585:0
    #8 0x1c2ba774 in ChromeBrowserMainExtraPartsAsh::PostMainMessageLoopRun() chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc:167:0
    #9 0xf69ca8d in ChromeBrowserMainParts::PostMainMessageLoopRun() chrome/browser/chrome_browser_main.cc:1948:29
    #10 0x6047fc8 in chromeos::ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() chrome/browser/chromeos/chrome_browser_main_chromeos.cc:1132:32
    #11 0x8e976d2 in content::BrowserMainLoop::ShutdownThreadsAndCleanUp() content/browser/browser_main_loop.cc:1221:13
    #12 0x8ea2b09 in content::BrowserMainRunnerImpl::Shutdown() content/browser/browser_main_runner.cc:208:19
    #13 0x8e812fa in content::BrowserMain(content::MainFunctionParams const&) content/browser/browser_main.cc:48:16
    #14 0xef181a0 in content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) content/app/content_main_runner.cc:425:14
    #15 0xef1b0bf in content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:703:12
    #16 0x17b48a92 in service_manager::Main(service_manager::MainParams const&) services/service_manager/embedder/main.cc:469:29
    #17 0xef14897 in content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:10
    #18 0x10cc6dd9 in content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:272:3
    #19 0xf55dd6e in InProcessBrowserTest::SetUp() chrome/test/base/in_process_browser_test.cc:267:20
    #20 0x4db800a in chromeos::DemoAppLauncherTest::SetUp() chrome/browser/chromeos/login/demo_mode/demo_app_launcher_browsertest.cc:82:27
    #21 0x6d99a6b in HandleExceptionsInMethodIfSupported<testing::Test, void> third_party/googletest/src/googletest/src/gtest.cc:2456:12
    #22 0x6d99a6b in testing::Test::Run() third_party/googletest/src/googletest/src/gtest.cc:2468:0
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/b/s/w/ir/out/Release/browser_tests+0x174c40a2)
Exiting
[1:10:0100/000000.399335:ERROR:broker_posix.cc(40)] Recvmsg error: Connection reset by peer (104)

Original change's description:
> cros: Unify cash and mash ChromeLauncherController management.
> 
> Refine ChromeLauncherController lifetime management.
> (always use the new ChromeLauncherControllerInitializer)
> 
> Remove ShellDelegate::ShelfInit and ShelfShutdown.
> Remove ShelfInitializer (tests/shell init via default prefs).
> Cleanup (std::make_unique, TestShellDelegate usage, etc)
> Reorder display update test expectations (unclear what changed...)
> 
> Bug:  557406 ,  681072 
> Change-Id: I5cd35b711c1d96761ac2204f7ba738eef58829ab
> Reviewed-on: https://chromium-review.googlesource.com/667838
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Commit-Queue: Michael Wasserman <msw@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#502376}

TBR=jamescook@chromium.org,msw@chromium.org,oshima@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  557406 ,  681072 
Change-Id: Ib4c82e867423df08a2fa4218788bd6316ef29284
Reviewed-on: https://chromium-review.googlesource.com/670684
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502538}
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/ash/display/display_manager_unittest.cc
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/ash/mus/shell_delegate_mus.h
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/ash/shell.cc
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/ash/shell_delegate.h
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/ash/test_shell_delegate.cc
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/ash/test_shell_delegate.h
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/chrome/browser/ui/ash/chrome_shell_delegate.h
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
[modify] https://crrev.com/ac26b921c785cd45429ebd49465eeefbf6b34827/chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Sep 18 2017

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

commit 4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f
Author: Mike Wasserman <msw@chromium.org>
Date: Mon Sep 18 20:59:27 2017

Reland cros: Unify cash and mash ChromeLauncherController management.

The first CL was reverted for msan failures:
https://chromium-review.googlesource.com/c/chromium/src/+/670684
This CL refines destruction timing for shelf item delegates.
(destroy with ChromeLauncherController and ShelfController)

ORIGINAL DESCRIPTION:

Refine ChromeLauncherController lifetime management.
(always use the new ChromeLauncherControllerInitializer)

Remove ShellDelegate::ShelfInit and ShelfShutdown.
Remove ShelfInitializer (tests/shell init via default prefs).
Cleanup (std::make_unique, TestShellDelegate usage, etc)
Reorder display update test expectations (unclear what changed...)

Bug:  557406 ,  681072 
Change-Id: I64eb5be117ed8d78e4fb685e51f18fd40ec358cd
Reviewed-on: https://chromium-review.googlesource.com/667838
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502376}
Reviewed-on: https://chromium-review.googlesource.com/671488
Cr-Commit-Position: refs/heads/master@{#502671}
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/display/display_manager_unittest.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/mus/shell_delegate_mus.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/mus/shell_delegate_mus.h
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/shelf/shelf_controller.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/shell.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/shell/shell_delegate_impl.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/shell/shell_delegate_impl.h
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/shell_delegate.h
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/test_shell_delegate.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/ash/test_shell_delegate.h
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/chrome/browser/ui/ash/chrome_shell_delegate.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/chrome/browser/ui/ash/chrome_shell_delegate.h
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
[modify] https://crrev.com/4cc9f40bc5fa6e06082afd28d06b2b6bec8df68f/chrome/browser/ui/ash/launcher/launcher_context_menu_unittest.cc

Comment 19 by msw@chromium.org, Sep 19 2017

Status: Fixed (was: Started)
I'm calling this fixed; remaining minor defects are/may be marked as blocking  Issue 557406 .
I'll be landing chrome/ash shelf model synchronization as an experiment for cash soon:
  https://chromium-review.googlesource.com/c/chromium/src/+/673366

Comment 20 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 21 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment