New issue
Advanced search Search tips

Issue 679925 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Rename WmShell::CreateShelf and similar

Project Member Reported by jamescook@chromium.org, Jan 11 2017

Issue description

This is confusing because it doesn't create the "shelf". WmShelf and ShelfWidget already exist when it is used. "CreateShelf" creates the shelf view and loads the icons.

It should probably be called CreateShelfView or InitializeShelf or PopulateShelf().

msw, thoughts on names?

 

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

Any of those names sgtm, I'd also rank them in the order you gave.
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 19 2017

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

commit 072742fba50cd351ef4b7d7b4b73f348b157ddef
Author: jamescook <jamescook@chromium.org>
Date: Thu Jan 19 17:58:14 2017

chromeos: Rename various CreateShelf methods to CreateShelfView

Shelf construction is confusing: WmShelf is created immediately for each
RootWindowController and ShelfWidget shortly thereafter. The ShelfView, on
the other hand, is only created after login.

Attempt to clarify the situation by renaming a bunch of "CreateShelf"
methods to CreateShelfView. This exposed a couple other refactors that I
tagged with TODOs.

No functional changes.

BUG= 679925 
TEST=ash_unittests
TBR=xdai@chromium.org

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

[modify] https://crrev.com/072742fba50cd351ef4b7d7b4b73f348b157ddef/ash/common/shelf/wm_shelf.cc
[modify] https://crrev.com/072742fba50cd351ef4b7d7b4b73f348b157ddef/ash/common/shelf/wm_shelf.h
[modify] https://crrev.com/072742fba50cd351ef4b7d7b4b73f348b157ddef/ash/common/test/test_session_state_delegate.cc
[modify] https://crrev.com/072742fba50cd351ef4b7d7b4b73f348b157ddef/ash/common/wm_shell.cc
[modify] https://crrev.com/072742fba50cd351ef4b7d7b4b73f348b157ddef/ash/common/wm_shell.h
[modify] https://crrev.com/072742fba50cd351ef4b7d7b4b73f348b157ddef/ash/root_window_controller.cc
[modify] https://crrev.com/072742fba50cd351ef4b7d7b4b73f348b157ddef/ash/root_window_controller.h
[modify] https://crrev.com/072742fba50cd351ef4b7d7b4b73f348b157ddef/ash/shell/content/client/shell_browser_main_parts.cc
[modify] https://crrev.com/072742fba50cd351ef4b7d7b4b73f348b157ddef/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager_browsertest.cc

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)

Sign in to add a comment