chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc - Issue 2833173002: mash: Support ShelfModel access in Chrome.

Unified Diff: chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

Issue 2833173002: mash: Support ShelfModel access in Chrome. (Closed)

Patch Set: Refine init pattern; add AppList item in ShelfModel ctor. Created 3 years, 7 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc ('K') | « chrome/browser/ui/ash/launcher/chrome_launcher_controller_browsertest.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

index 4b9706709d1a71c767af509263668c8acc2a93c1..eb218c822125d0de755e7144d3a20edc3e2a3e22 100644

--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

@@ -42,6 +42,7 @@

#include "base/values.h"

#include "build/build_config.h"

#include "chrome/browser/chromeos/arc/arc_util.h"

+#include "chrome/browser/chromeos/ash_config.h"

#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"

#include "chrome/browser/chromeos/login/users/scoped_user_manager_enabler.h"

#include "chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.h"

@@ -127,8 +128,6 @@ constexpr char kOfflineGmailUrl[] = "https://mail.google.com/mail/mu/u";

constexpr char kGmailUrl[] = "https://mail.google.com/mail/u";

constexpr char kGmailLaunchURL[] = "https://mail.google.com/mail/ca";

-constexpr char kAppListId[] = "jlfapfmkapbjlfbpjedlinehodkccjee";

// An extension prefix.

constexpr char kCrxAppPrefix[] = "_crx_";

@@ -162,6 +161,9 @@ class TestShelfModelObserver : public ash::ShelfModelObserver {

last_index_ = target_index;

}

+ void ShelfItemDelegateChanged(const ash::ShelfID&,

+ ash::ShelfItemDelegate*) override {}

void clear_counts() {

added_ = 0;

removed_ = 0;

@@ -275,7 +277,7 @@ class TestV2AppLauncherItemController : public ash::ShelfItemDelegate {

DISALLOW_COPY_AND_ASSIGN(TestV2AppLauncherItemController);

};

-// A test ShelfController implementation that tracks alignment and auto-hide.

+// A test ShelfController implementation that tracks state and function calls.

class TestShelfController : public ash::mojom::ShelfController {

public:

TestShelfController() : binding_(this) {}

@@ -287,6 +289,9 @@ class TestShelfController : public ash::mojom::ShelfController {

size_t alignment_change_count() const { return alignment_change_count_; }

size_t auto_hide_change_count() const { return auto_hide_change_count_; }

+ size_t added_count() const { return added_count_; }

+ size_t removed_count() const { return removed_count_; }

ash::mojom::ShelfControllerPtr CreateInterfacePtrAndBind() {

return binding_.CreateInterfacePtrAndBind();

}

@@ -308,12 +313,12 @@ class TestShelfController : public ash::mojom::ShelfController {

auto_hide_ = auto_hide;

observer_->OnAutoHideBehaviorChanged(auto_hide_, display_id);

}

- void PinItem(

- const ash::ShelfItem& item,

- ash::mojom::ShelfItemDelegateAssociatedPtrInfo delegate) override {}

- void UnpinItem(const std::string& app_id) override {}

- void SetItemImage(const std::string& app_id, const SkBitmap& image) override {

- }

+ void AddShelfItem(int32_t, const ash::ShelfItem&) override { added_count_++; }

+ void RemoveShelfItem(const ash::ShelfID&) override { removed_count_++; }

+ void MoveShelfItem(const ash::ShelfID&, int32_t) override {}

+ void UpdateShelfItem(const ash::ShelfItem&) override {}

+ void SetShelfItemDelegate(const ash::ShelfID&,

+ ash::mojom::ShelfItemDelegatePtr) override {}

private:

ash::ShelfAlignment alignment_ = ash::SHELF_ALIGNMENT_BOTTOM_LOCKED;

@@ -322,6 +327,9 @@ class TestShelfController : public ash::mojom::ShelfController {

size_t alignment_change_count_ = 0;

size_t auto_hide_change_count_ = 0;

+ size_t added_count_ = 0;

+ size_t removed_count_ = 0;

ash::mojom::ShelfObserverAssociatedPtr observer_;

mojo::Binding<ash::mojom::ShelfController> binding_;

@@ -373,30 +381,22 @@ class TestChromeLauncherController : public ChromeLauncherController {

// TODO(msw): Avoid relying on TestShellDelegate's ShelfInitializer.

class ChromeLauncherTestShellDelegate : public ash::test::TestShellDelegate {

public:

- ChromeLauncherTestShellDelegate() {}

- // Create a ChromeLauncherController instance.

- ChromeLauncherController* CreateLauncherController(Profile* profile) {

- launcher_controller_ = base::MakeUnique<ChromeLauncherController>(

- profile, ash::Shell::Get()->shelf_model());

- return launcher_controller_.get();

- }

+ explicit ChromeLauncherTestShellDelegate(ash::ShelfModel* shelf_model)

+ : shelf_model_(shelf_model) {}

// Create a TestChromeLauncherController instance.

- TestChromeLauncherController* CreateTestLauncherController(Profile* profile) {

- auto controller = base::MakeUnique<TestChromeLauncherController>(

- profile, ash::Shell::Get()->shelf_model());

- TestChromeLauncherController* controller_weak = controller.get();

- launcher_controller_ = std::move(controller);

- launcher_controller_->Init();

- return controller_weak;

+ TestChromeLauncherController* CreateLauncherController(Profile* profile) {

+ launcher_controller_ =

+ base::MakeUnique<TestChromeLauncherController>(profile, shelf_model_);

+ return launcher_controller_.get();

}

// ash::test::TestShellDelegate:

void ShelfShutdown() override { launcher_controller_.reset(); }

private:

- std::unique_ptr<ChromeLauncherController> launcher_controller_;

+ ash::ShelfModel* shelf_model_;

+ std::unique_ptr<TestChromeLauncherController> launcher_controller_;

DISALLOW_COPY_AND_ASSIGN(ChromeLauncherTestShellDelegate);

};

@@ -414,7 +414,7 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

app_list::AppListSyncableServiceFactory::SetUseInTesting();

- shell_delegate_ = new ChromeLauncherTestShellDelegate();

+ shell_delegate_ = new ChromeLauncherTestShellDelegate(&model_);

ash_test_helper()->set_test_shell_delegate(shell_delegate_);

BrowserWithTestWindowTest::SetUp();

@@ -425,9 +425,8 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

ASSERT_TRUE(profile_manager_->SetUp());

}

- model_ = ash::Shell::Get()->shelf_controller()->model();

model_observer_.reset(new TestShelfModelObserver);

- model_->AddObserver(model_observer_.get());

+ model_.AddObserver(model_observer_.get());

base::DictionaryValue manifest;

manifest.SetString(extensions::manifest_keys::kName,

@@ -530,7 +529,7 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

base::MakeUnique<TestV2AppLauncherItemController>(app_id);

test_controller_ = controller.get();

ash::ShelfID id = launcher_controller_->InsertAppLauncherItem(

- std::move(controller), ash::STATUS_RUNNING, model_->item_count(),

+ std::move(controller), ash::STATUS_RUNNING, model_.item_count(),

ash::TYPE_APP);

DCHECK(launcher_controller_->IsPlatformApp(id));

}

@@ -578,7 +577,7 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

void TearDown() override {

arc_test_.TearDown();

- model_->RemoveObserver(model_observer_.get());

+ model_.RemoveObserver(model_observer_.get());

model_observer_.reset();

launcher_controller_ = nullptr;

BrowserWithTestWindowTest::TearDown();

@@ -596,13 +595,6 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

CreateBrowser(profile, Browser::TYPE_TABBED, false, browser_window));

}

- void AddAppListLauncherItem() {

- ash::ShelfItem app_list;

- app_list.id = ash::ShelfID(kAppListId);

- app_list.type = ash::TYPE_APP_LIST;

- model_->Add(app_list);

- }

// Create a launcher controller instance, owned by the test shell delegate.

// Returns a pointer to the uninitialized controller.

ChromeLauncherController* CreateLauncherController() {

@@ -633,9 +625,10 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

ChromeLauncherController* RecreateLauncherController() {

// Destroy any existing controller first; only one may exist at a time.

ResetLauncherController();

- while (model_->item_count() > 0)

- model_->RemoveItemAt(0);

- AddAppListLauncherItem();

+ DCHECK_GE(model_.item_count(), 1);

+ DCHECK_EQ(ash::kAppListId, model_.items[0].id.app_id);

+ while (model_.item_count() > 1)

+ model_.RemoveItemAt(1);

return CreateLauncherController();

}

@@ -828,7 +821,7 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

void GetPinnedAppIds(ChromeLauncherController* controller,

std::vector<std::string>* app_ids) {

app_ids->clear();

- for (const auto& item : model_->items()) {

+ for (const auto& item : model_.items()) {

if (item.type == ash::TYPE_PINNED_APP)

app_ids->push_back(item.id.app_id);

}

@@ -840,14 +833,14 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

// pinned V2 app will start with a '*' + small letter.

std::string GetPinnedAppStatus() {

std::string result;

- for (int i = 0; i < model_->item_count(); i++) {

+ for (int i = 0; i < model_.item_count(); i++) {

if (!result.empty())

result.append(", ");

- switch (model_->items()[i].type) {

+ switch (model_.items()[i].type) {

case ash::TYPE_APP: {

- if (launcher_controller_->IsPlatformApp(model_->items()[i].id))

+ if (launcher_controller_->IsPlatformApp(model_.items()[i].id))

result += "*";

- const std::string& app = model_->items()[i].id.app_id;

+ const std::string& app = model_.items()[i].id.app_id;

EXPECT_FALSE(launcher_controller_->IsAppPinned(app));

if (app == extension1_->id()) {

result += "app1";

@@ -873,9 +866,9 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

break;

}

case ash::TYPE_PINNED_APP: {

- if (launcher_controller_->IsPlatformApp(model_->items()[i].id))

+ if (launcher_controller_->IsPlatformApp(model_.items()[i].id))

result += "*";

- const std::string& app = model_->items()[i].id.app_id;

+ const std::string& app = model_.items()[i].id.app_id;

EXPECT_TRUE(launcher_controller_->IsAppPinned(app));

if (app == extension1_->id()) {

result += "App1";

@@ -1050,7 +1043,7 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {

ChromeLauncherController* launcher_controller_ = nullptr;

ChromeLauncherTestShellDelegate* shell_delegate_ = nullptr;

std::unique_ptr<TestShelfModelObserver> model_observer_;

- ash::ShelfModel* model_ = nullptr;

+ ash::ShelfModel model_;

std::unique_ptr<TestingProfileManager> profile_manager_;

// |item_delegate_manager_| owns |test_controller_|.

@@ -1354,7 +1347,7 @@ INSTANTIATE_TEST_CASE_P(,

TEST_F(ChromeLauncherControllerTest, DefaultApps) {

InitLauncherController();

// The model should only contain the browser shortcut and app list items.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));

@@ -1427,10 +1420,10 @@ TEST_P(ChromeLauncherControllerWithArcTest, ArcAppPinCrossPlatformWorkflow) {

GetPinnedAppStatus());

// Now move pins on ARC enabled platform.

- model_->Move(1, 4);

- model_->Move(3, 1);

- model_->Move(3, 5);

- model_->Move(4, 2);

+ model_.Move(1, 4);

+ model_.Move(3, 1);

+ model_.Move(3, 5);

+ model_.Move(4, 2);

EXPECT_EQ("AppList, App2, Fake App 1, Chrome, App1, Fake App 0, App3",

GetPinnedAppStatus());

@@ -1460,7 +1453,7 @@ TEST_P(ChromeLauncherControllerWithArcTest, ArcAppPinCrossPlatformWorkflow) {

EXPECT_EQ("AppList, App2, Chrome, App1, App3", GetPinnedAppStatus());

// Now move/remove pins on ARC disabled platform.

- model_->Move(4, 2);

+ model_.Move(4, 2);

launcher_controller_->UnpinAppWithID(extension2_->id());

EXPECT_EQ("AppList, App3, Chrome, App1", GetPinnedAppStatus());

EnablePlayStore(true);

@@ -1698,76 +1691,76 @@ TEST_F(ChromeLauncherControllerTest, RestoreDefaultAppsResyncOrder) {

TEST_F(ChromeLauncherControllerTest, V1AppRunActivateClose) {

InitLauncherController();

// The model should only contain the browser shortcut and app list items.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_EQ(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Reporting that the app is running should create a new shelf item.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_RUNNING, model_.items()[2].status);

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Reporting an active status should just update the existing item.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_ACTIVE);

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::STATUS_ACTIVE, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::STATUS_ACTIVE, model_.items()[2].status);

// Reporting that the app is closed should remove its shelf item.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_EQ(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Reporting that the app is closed again should have no effect.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

}

// Test the V1 app interaction flow: pin it, run it, close it, unpin it.

TEST_F(ChromeLauncherControllerTest, V1AppPinRunCloseUnpin) {

InitLauncherController();

// The model should only contain the browser shortcut and app list items.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_EQ(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Pinning the app should create a new shelf item.

launcher_controller_->PinAppWithID(extension1_->id());

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_CLOSED, model_.items()[2].status);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Reporting that the app is running should just update the existing item.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_RUNNING, model_.items()[2].status);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Reporting that the app is closed should just update the existing item.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_CLOSED, model_.items()[2].status);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Unpinning the app should remove its shelf item.

launcher_controller_->UnpinAppWithID(extension1_->id());

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_EQ(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

@@ -1777,41 +1770,41 @@ TEST_F(ChromeLauncherControllerTest, V1AppPinRunCloseUnpin) {

TEST_F(ChromeLauncherControllerTest, V1AppRunPinCloseUnpin) {

InitLauncherController();

// The model should only contain the browser shortcut and app list items.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_EQ(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Reporting that the app is running should create a new shelf item.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_RUNNING, model_.items()[2].status);

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Pinning the app should just update the existing item.

launcher_controller_->PinAppWithID(extension1_->id());

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_RUNNING, model_.items()[2].status);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Reporting that the app is closed should just update the existing item.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_CLOSED, model_.items()[2].status);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Unpinning the app should remove its shelf item.

launcher_controller_->UnpinAppWithID(extension1_->id());

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_EQ(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

@@ -1821,41 +1814,41 @@ TEST_F(ChromeLauncherControllerTest, V1AppRunPinCloseUnpin) {

TEST_F(ChromeLauncherControllerTest, V1AppPinRunUnpinClose) {

InitLauncherController();

// The model should only contain the browser shortcut and app list items.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_EQ(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Pinning the app should create a new shelf item.

launcher_controller_->PinAppWithID(extension1_->id());

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_CLOSED, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_CLOSED, model_.items()[2].status);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Reporting that the app is running should just update the existing item.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_RUNNING, model_.items()[2].status);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Unpinning the app should just update the existing item.

launcher_controller_->UnpinAppWithID(extension1_->id());

- EXPECT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_APP, model_->items()[2].type);

- EXPECT_EQ(ash::STATUS_RUNNING, model_->items()[2].status);

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::STATUS_RUNNING, model_.items()[2].status);

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_NE(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

// Reporting that the app is closed should remove its shelf item.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_EQ(nullptr,

launcher_controller_->GetItem(ash::ShelfID(extension1_->id())));

@@ -1865,13 +1858,13 @@ TEST_F(ChromeLauncherControllerTest, V1AppPinRunUnpinClose) {

TEST_F(ChromeLauncherControllerTest, CheckRunningV1AppOrder) {

InitLauncherController();

// The model should only contain the browser shortcut and app list items.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// Add a few running applications.

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_RUNNING);

launcher_controller_->SetV1AppStatus(extension2_->id(), ash::STATUS_RUNNING);

launcher_controller_->SetV1AppStatus(extension3_->id(), ash::STATUS_RUNNING);

- EXPECT_EQ(5, model_->item_count());

+ EXPECT_EQ(5, model_.item_count());

// Note that this not only checks the order of applications but also the

// running type.

EXPECT_EQ("AppList, Chrome, app1, app2, app3", GetPinnedAppStatus());

@@ -1883,7 +1876,7 @@ TEST_F(ChromeLauncherControllerTest, CheckRunningV1AppOrder) {

// Switch some items and check that restoring a user which was not yet

// remembered changes nothing.

- model_->Move(2, 3);

+ model_.Move(2, 3);

EXPECT_EQ("AppList, Chrome, app2, app1, app3", GetPinnedAppStatus());

const AccountId second_fake_account_id(

AccountId::FromUserEmail("second-fake-user@fake.com"));

@@ -1896,7 +1889,7 @@ TEST_F(ChromeLauncherControllerTest, CheckRunningV1AppOrder) {

// Switch again some items and even delete one - making sure that the missing

// item gets properly handled.

- model_->Move(3, 4);

+ model_.Move(3, 4);

launcher_controller_->SetV1AppStatus(extension1_->id(), ash::STATUS_CLOSED);

EXPECT_EQ("AppList, Chrome, app3, app2", GetPinnedAppStatus());

RestoreUnpinnedRunningApplicationOrder(current_account_id);

@@ -1951,7 +1944,7 @@ TEST_P(ChromeLauncherControllerWithArcTest, ArcDeferredLaunch) {

// We activated arc_app_id1 twice but expect one close for item controller

// stops launching request.

ash::ShelfItemDelegate* item_delegate =

- model_->GetShelfItemDelegate(shelf_id_app_1);

+ model_.GetShelfItemDelegate(shelf_id_app_1);

ASSERT_NE(nullptr, item_delegate);

item_delegate->Close();

base::RunLoop().RunUntilIdle();

@@ -2007,7 +2000,7 @@ TEST_P(ChromeLauncherControllerWithArcTest, ArcDeferredLaunchForActiveApp) {

// Play Store app is ARC app that might be represented by native Chrome

// platform app.

- model_->SetShelfItemDelegate(

+ model_.SetShelfItemDelegate(

shelf_id,

base::MakeUnique<ExtensionAppWindowLauncherItemController>(shelf_id));

launcher_controller_->SetItemStatus(shelf_id, ash::STATUS_RUNNING);

@@ -2324,7 +2317,7 @@ TEST_P(ChromeLauncherControllerWithArcTest, ArcCustomAppIcon) {

std::string());

EXPECT_TRUE(launcher_controller_->GetItem(ash::ShelfID(arc_app_id)));

ash::ShelfItemDelegate* item_delegate =

- model_->GetShelfItemDelegate(ash::ShelfID(arc_app_id));

+ model_.GetShelfItemDelegate(ash::ShelfID(arc_app_id));

ASSERT_TRUE(item_delegate);

// No custom icon set. Acitivating windows should not change icon.

@@ -2365,12 +2358,12 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,

// Create a browser item in the LauncherController.

InitLauncherController();

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

{

// Create a "windowed gmail app".

std::unique_ptr<V1App> v1_app(

CreateRunningV1App(profile(), extension_misc::kGmailAppId, kGmailUrl));

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

// After switching to a second user the item should be gone.

std::string user2 = "user2";

@@ -2380,14 +2373,14 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,

const AccountId account_id(

multi_user_util::GetAccountIdFromProfile(profile()));

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// After switching back the item should be back.

SwitchActiveUser(account_id);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

// Note we destroy now the gmail app with the closure end.

}

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

}

// Check edge cases with multi profile V1 apps in the shelf.

@@ -2407,23 +2400,23 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,

// Create a "windowed gmail app".

std::unique_ptr<V1App> v1_app(

CreateRunningV1App(profile2, extension_misc::kGmailAppId, kGmailUrl));

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// However - switching to the user should show it.

SwitchActiveUser(account_id2);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

// Second test: Remove the app when the user is not active and see that it

// works.

SwitchActiveUser(account_id);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// Note: the closure ends and the browser will go away.

}

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

SwitchActiveUser(account_id);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

}

// Check edge case where a visiting V1 app gets closed (crbug.com/321374).

@@ -2446,28 +2439,28 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,

// Create a "windowed gmail app".

std::unique_ptr<V1App> v1_app(CreateRunningV1App(

profile(), extension_misc::kGmailAppId, kGmailLaunchURL));

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

// Transfer the app to the other screen and switch users.

manager->ShowWindowForUser(v1_app->browser()->window()->GetNativeWindow(),

account_id2);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

}

// After the app was destroyed, switch back. (which caused already a crash).

SwitchActiveUser(account_id);

// Create the same app again - which was also causing the crash.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

{

// Create a "windowed gmail app".

std::unique_ptr<V1App> v1_app(CreateRunningV1App(

profile(), extension_misc::kGmailAppId, kGmailLaunchURL));

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

}

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

}

// Check edge cases with multi profile V1 apps in the shelf.

@@ -2488,23 +2481,23 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,

// Create a "windowed gmail app".

std::unique_ptr<V1App> v1_app(

CreateRunningV1App(profile(), extension_misc::kGmailAppId, kGmailUrl));

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// However - switching to the user should show it.

SwitchActiveUser(account_id);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

// Second test: Remove the app when the user is not active and see that it

// works.

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

v1_app.reset();

}

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

SwitchActiveUser(account_id);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

}

// Check that activating an item which is on another user's desktop, will bring

@@ -2747,17 +2740,17 @@ TEST_F(ChromeLauncherControllerTest, Policy) {

// Only |extension1_| should get pinned. |extension2_| is specified but not

// installed, and |extension3_| is part of the default set, but that shouldn't

// take effect when the policy override is in place.

- ASSERT_EQ(3, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[1].type);

+ ASSERT_EQ(3, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[1].type);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension2_->id()));

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));

// Installing |extension2_| should add it to the launcher.

extension_service_->AddExtension(extension2_.get());

- ASSERT_EQ(4, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[1].type);

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

+ ASSERT_EQ(4, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[1].type);

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension2_->id()));

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));

@@ -2767,8 +2760,8 @@ TEST_F(ChromeLauncherControllerTest, Policy) {

policy_value.Remove(0, NULL);

profile()->GetTestingPrefService()->SetManagedPref(

prefs::kPolicyPinnedLauncherApps, policy_value.CreateDeepCopy());

- EXPECT_EQ(4, model_->item_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

+ EXPECT_EQ(4, model_.item_count());

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension1_->id()));

EXPECT_TRUE(launcher_controller_->IsAppPinned(extension2_->id()));

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));

@@ -3039,7 +3032,7 @@ TEST_F(ChromeLauncherControllerTest, V1AppMenuGeneration) {

InitLauncherControllerWithBrowser();

// The model should only contain the browser shortcut and app list items.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

EXPECT_FALSE(launcher_controller_->IsAppPinned(extension3_->id()));

// Installing |extension3_| pins it to the launcher.

@@ -3152,19 +3145,19 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,

const AccountId account_id2(

multi_user_util::GetAccountIdFromProfile(profile2));

// Check that there is a browser and a app launcher.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// Add a v2 app.

V2App v2_app(profile(), extension1_.get());

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

// After switching users the item should go away.

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// And it should come back when switching back.

SwitchActiveUser(account_id);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

}

// Check that V2 applications are creating items properly in edge cases:

@@ -3180,35 +3173,35 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,

const AccountId account_id2(

multi_user_util::GetAccountIdFromProfile(profile2));

// Check that there is a browser and a app launcher.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// Switch to an inactive user.

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// Add the v2 app to the inactive user and check that no item was added to

// the launcher.

{

V2App v2_app(profile(), extension1_.get());

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// Switch to the primary user and check that the item is shown.

SwitchActiveUser(account_id);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

// Switch to the second user and check that the item goes away - even if the

// item gets closed.

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

}

// After the application was killed there should be still 2 items.

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

// Switching then back to the default user should not show the additional item

// anymore.

SwitchActiveUser(account_id);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

}

// Check that V2 applications will be made visible on the target desktop if

@@ -3294,64 +3287,64 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,

const AccountId account_id2(

multi_user_util::GetAccountIdFromProfile(profile2));

SwitchActiveUser(account_id);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

V2App v2_app_1(profile(), extension1_.get());

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

{

// Hide and show the app.

v2_app_1.window()->Hide();

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

v2_app_1.window()->Show(extensions::AppWindow::SHOW_ACTIVE);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

}

{

// Switch user, hide and show the app and switch back.

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

v2_app_1.window()->Hide();

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

v2_app_1.window()->Show(extensions::AppWindow::SHOW_ACTIVE);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

SwitchActiveUser(account_id);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

}

{

// Switch user, hide the app, switch back and then show it again.

SwitchActiveUser(account_id2);

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

v2_app_1.window()->Hide();

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

SwitchActiveUser(account_id);

// The following expectation does not work in current impl. It was working

// before because MultiUserWindowManagerChromeOS is not attached to user

// associated with profile() hence not actually handling windows for the

// user. It is a real bug. See http://crbug.com/693634

- // EXPECT_EQ(2, model_->item_count());

+ // EXPECT_EQ(2, model_.item_count());

v2_app_1.window()->Show(extensions::AppWindow::SHOW_ACTIVE);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

}

{

// Create a second app, hide and show it and then hide both apps.

V2App v2_app_2(profile(), extension1_.get());

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

v2_app_2.window()->Hide();

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

v2_app_2.window()->Show(extensions::AppWindow::SHOW_ACTIVE);

- EXPECT_EQ(3, model_->item_count());

+ EXPECT_EQ(3, model_.item_count());

v2_app_1.window()->Hide();

v2_app_2.window()->Hide();

- EXPECT_EQ(2, model_->item_count());

+ EXPECT_EQ(2, model_.item_count());

}

@@ -3376,8 +3369,7 @@ TEST_F(ChromeLauncherControllerTest, V1AppMenuExecution) {

item_gmail.id = gmail_id;

base::string16 two_menu_items[] = {title1, title2};

CheckAppMenu(launcher_controller_, item_gmail, 2, two_menu_items);

- ash::ShelfItemDelegate* item_delegate =

- model_->GetShelfItemDelegate(gmail_id);

+ ash::ShelfItemDelegate* item_delegate = model_.GetShelfItemDelegate(gmail_id);

ASSERT_TRUE(item_delegate);

EXPECT_EQ(1, browser()->tab_strip_model()->active_index());

// Execute the second item in the menu, after the title and two separators,

@@ -3425,8 +3417,7 @@ TEST_F(ChromeLauncherControllerTest, V1AppMenuDeletionExecution) {

base::string16 two_menu_items[] = {title1, title2};

CheckAppMenu(launcher_controller_, item_gmail, 2, two_menu_items);

- ash::ShelfItemDelegate* item_delegate =

- model_->GetShelfItemDelegate(gmail_id);

+ ash::ShelfItemDelegate* item_delegate = model_.GetShelfItemDelegate(gmail_id);

ASSERT_TRUE(item_delegate);

int tabs = browser()->tab_strip_model()->count();

// Activate the proper tab through the menu item.

@@ -3446,48 +3437,6 @@ TEST_F(ChromeLauncherControllerTest, V1AppMenuDeletionExecution) {

}

-// Tests that panels create launcher items correctly

-TEST_F(ChromeLauncherControllerTest, AppPanels) {

James Cook 2017/05/30 22:47:48 I presume this is going away because we're getting

msw 2017/05/31 16:55:22 Replaced by ShelfWindowWatcherTest.ItemIcon.

- InitLauncherController();

- model_observer_->clear_counts();

- const std::string app_id = extension1_->id();

- // app_icon_loader is owned by ChromeLauncherController.

- TestAppIconLoaderImpl* app_icon_loader = new TestAppIconLoaderImpl();

- app_icon_loader->AddSupportedApp(app_id);

- SetAppIconLoader(std::unique_ptr<AppIconLoader>(app_icon_loader));

- // Make an app panel; the ShelfItem is added by ash::ShelfWindowWatcher.

- std::unique_ptr<V2App> app_panel1 = base::MakeUnique<V2App>(

- profile(), extension1_.get(), extensions::AppWindow::WINDOW_TYPE_PANEL);

- EXPECT_TRUE(app_panel1->window()->GetNativeWindow()->IsVisible());

- int panel_index = model_observer_->last_index();

- EXPECT_EQ(1, model_observer_->added());

- EXPECT_EQ(1, app_icon_loader->fetch_count());

- model_observer_->clear_counts();

- // App panels should have a separate identifier than the app id

- EXPECT_FALSE(launcher_controller_->GetItem(ash::ShelfID(app_id)));

- // Setting the app image should not change the panel, which has a window icon.

- gfx::ImageSkia image;

- launcher_controller_->OnAppImageUpdated(app_id, image);

- EXPECT_EQ(0, model_observer_->changed());

- model_observer_->clear_counts();

- // Make a second app panel and verify that it gets the same index as the first

- // panel, being added to the left of the existing panel.

- std::unique_ptr<V2App> app_panel2 = base::MakeUnique<V2App>(

- profile(), extension2_.get(), extensions::AppWindow::WINDOW_TYPE_PANEL);

- EXPECT_EQ(panel_index, model_observer_->last_index());

- EXPECT_EQ(1, model_observer_->added());

- model_observer_->clear_counts();

- app_panel1.reset();

- app_panel2.reset();

- EXPECT_EQ(2, model_observer_->removed());

// Tests that the Gmail extension matches more than the app itself claims with

// the manifest file.

TEST_F(ChromeLauncherControllerTest, GmailMatching) {

@@ -3546,8 +3495,8 @@ TEST_F(ChromeLauncherControllerTest, PersistLauncherItemPositions) {

TestLauncherControllerHelper* helper = new TestLauncherControllerHelper;

SetLauncherControllerHelper(helper);

- EXPECT_EQ(ash::TYPE_APP_LIST, model_->items()[0].type);

- EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_->items()[1].type);

+ EXPECT_EQ(ash::TYPE_APP_LIST, model_.items()[0].type);

+ EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_.items()[1].type);

TabStripModel* tab_strip_model = browser()->tab_strip_model();

EXPECT_EQ(0, tab_strip_model->count());

@@ -3562,17 +3511,17 @@ TEST_F(ChromeLauncherControllerTest, PersistLauncherItemPositions) {

EXPECT_TRUE(launcher_controller_->IsAppPinned("1"));

launcher_controller_->PinAppWithID("2");

- EXPECT_EQ(ash::TYPE_APP_LIST, model_->items()[0].type);

- EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_->items()[1].type);

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[3].type);

+ EXPECT_EQ(ash::TYPE_APP_LIST, model_.items()[0].type);

+ EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_.items()[1].type);

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[3].type);

// Move browser shortcut item from index 1 to index 3.

- model_->Move(1, 3);

- EXPECT_EQ(ash::TYPE_APP_LIST, model_->items()[0].type);

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[1].type);

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_->items()[3].type);

+ model_.Move(1, 3);

+ EXPECT_EQ(ash::TYPE_APP_LIST, model_.items()[0].type);

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[1].type);

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_.items()[3].type);

RecreateLauncherController();

helper = new TestLauncherControllerHelper(profile());

@@ -3582,16 +3531,16 @@ TEST_F(ChromeLauncherControllerTest, PersistLauncherItemPositions) {

launcher_controller_->Init();

// Check ShelfItems are restored after resetting ChromeLauncherController.

- EXPECT_EQ(ash::TYPE_APP_LIST, model_->items()[0].type);

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[1].type);

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[2].type);

- EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_->items()[3].type);

+ EXPECT_EQ(ash::TYPE_APP_LIST, model_.items()[0].type);

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[1].type);

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[2].type);

+ EXPECT_EQ(ash::TYPE_BROWSER_SHORTCUT, model_.items()[3].type);

}

// Verifies pinned apps are persisted and restored.

TEST_F(ChromeLauncherControllerTest, PersistPinned) {

InitLauncherControllerWithBrowser();

- size_t initial_size = model_->items().size();

+ size_t initial_size = model_.items().size();

TabStripModel* tab_strip_model = browser()->tab_strip_model();

EXPECT_EQ(1, tab_strip_model->count());

@@ -3607,12 +3556,12 @@ TEST_F(ChromeLauncherControllerTest, PersistPinned) {

EXPECT_EQ(0, app_icon_loader->fetch_count());

launcher_controller_->PinAppWithID("1");

- const int app_index = model_->ItemIndexByID(ash::ShelfID("1"));

+ const int app_index = model_.ItemIndexByID(ash::ShelfID("1"));

EXPECT_EQ(1, app_icon_loader->fetch_count());

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[app_index].type);

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[app_index].type);

EXPECT_TRUE(launcher_controller_->IsAppPinned("1"));

EXPECT_FALSE(launcher_controller_->IsAppPinned("0"));

- EXPECT_EQ(initial_size + 1, model_->items().size());

+ EXPECT_EQ(initial_size + 1, model_.items().size());

RecreateLauncherController();

helper = new TestLauncherControllerHelper(profile());

@@ -3625,13 +3574,13 @@ TEST_F(ChromeLauncherControllerTest, PersistPinned) {

launcher_controller_->Init();

EXPECT_EQ(1, app_icon_loader->fetch_count());

- ASSERT_EQ(initial_size + 1, model_->items().size());

+ ASSERT_EQ(initial_size + 1, model_.items().size());

EXPECT_TRUE(launcher_controller_->IsAppPinned("1"));

EXPECT_FALSE(launcher_controller_->IsAppPinned("0"));

- EXPECT_EQ(ash::TYPE_PINNED_APP, model_->items()[app_index].type);

+ EXPECT_EQ(ash::TYPE_PINNED_APP, model_.items()[app_index].type);

launcher_controller_->UnpinAppWithID("1");

- ASSERT_EQ(initial_size, model_->items().size());

+ ASSERT_EQ(initial_size, model_.items().size());

}

TEST_F(ChromeLauncherControllerTest, MultipleAppIconLoaders) {

@@ -3795,7 +3744,7 @@ TEST_P(ChromeLauncherControllerWithArcTest, ShelfItemWithMultipleWindows) {

const std::string app_id = ArcAppTest::GetAppId(appinfo);

ash::ShelfItemDelegate* item_delegate =

- model_->GetShelfItemDelegate(ash::ShelfID(app_id));

+ model_.GetShelfItemDelegate(ash::ShelfID(app_id));

ASSERT_TRUE(item_delegate);

// Selecting the item will show its application menu. It does not change the

@@ -4135,7 +4084,7 @@ TEST_P(ChromeLauncherControllerArcDefaultAppsTest, PlayStoreDeferredLaunch) {

// Simulate click. This should schedule Play Store for deferred launch.

ash::ShelfItemDelegate* item_delegate =

- model_->GetShelfItemDelegate(ash::ShelfID(arc::kPlayStoreAppId));

+ model_.GetShelfItemDelegate(ash::ShelfID(arc::kPlayStoreAppId));

EXPECT_TRUE(item_delegate);

SelectItem(item_delegate);

EXPECT_TRUE(launcher_controller_->IsAppPinned(arc::kPlayStoreAppId));

@@ -4176,7 +4125,7 @@ TEST_F(ChromeLauncherControllerTest, CheckPositionConflict) {

// Move Chrome between App1 and App2.

// Note, move target_index is in context when moved element is removed from

// array first.

- model_->Move(1, 2);

+ model_.Move(1, 2);

EXPECT_EQ("AppList, App1, Chrome, App2, App3", GetPinnedAppStatus());

// Expect sync positions for only Chrome is updated and its resolution is

@@ -4234,7 +4183,8 @@ TEST_F(ChromeLauncherControllerTest, PrefsLoadedOnLogin) {

prefs->SetString(prefs::kShelfAutoHideBehavior, "Always");

TestChromeLauncherController* test_launcher_controller =

- shell_delegate_->CreateTestLauncherController(profile());

+ shell_delegate_->CreateLauncherController(profile());

+ test_launcher_controller->Init();

// Simulate login for the test controller.

test_launcher_controller->ReleaseProfile();

@@ -4254,7 +4204,8 @@ TEST_F(ChromeLauncherControllerTest, PrefsLoadedOnLogin) {

// Tests that the shelf controller's changes are not wastefully echoed back.

TEST_F(ChromeLauncherControllerTest, DoNotEchoShelfControllerChanges) {

TestChromeLauncherController* test_launcher_controller =

- shell_delegate_->CreateTestLauncherController(profile());

+ shell_delegate_->CreateLauncherController(profile());

+ test_launcher_controller->Init();

// Simulate login for the test controller.

test_launcher_controller->ReleaseProfile();

@@ -4292,3 +4243,67 @@ TEST_F(ChromeLauncherControllerTest, DoNotEchoShelfControllerChanges) {

EXPECT_EQ("Always", prefs->GetString(prefs::kShelfAutoHideBehaviorLocal));

EXPECT_EQ("Always", prefs->GetString(prefs::kShelfAutoHideBehavior));

}

+// Ensure Ash and Chrome ShelfModel changes are synchronized correctly.

+// In Ash's classic config, Chrome and Ash share the same ShelfModel instance.

+// In Ash's mash config, Chrome and Ash synchronize two ShelfModel instances.

+TEST_F(ChromeLauncherControllerTest, ShelfModelSyncsWithAsh) {

+ const bool is_mash = chromeos::GetAshConfig() == ash::Config::MASH;

+ // The ShelfModel constructor adds an AppList shelf item.

+ TestChromeLauncherController* launcher_controller =

+ shell_delegate_->CreateLauncherController(profile());

+ TestShelfController* shelf_controller =

+ launcher_controller->test_shelf_controller();

+ EXPECT_EQ(0u, shelf_controller->added_count());

+ EXPECT_EQ(0u, shelf_controller->removed_count());

+ EXPECT_EQ(1, model_.item_count());

+ EXPECT_EQ(ash::kAppListId, model_.items()[0].id.app_id);

+ // Init should add the browser shelf item to Chrome's ShelfModel.

+ // Ash's ShelfModel should have been notified about the browser item in mash.

+ launcher_controller->Init();

+ base::RunLoop().RunUntilIdle();

+ EXPECT_EQ(2, model_.item_count());

+ EXPECT_EQ(extension_misc::kChromeAppId, model_.items()[1].id.app_id);

+ EXPECT_EQ(is_mash ? 1u : 0u, shelf_controller->added_count());

James Cook 2017/05/30 22:47:48 As before, I think this would be clearer with two

msw 2017/05/31 16:55:22 This test is actually pretty pointless in classic

+ EXPECT_EQ(0u, shelf_controller->removed_count());

+ // Simulate adding a shelf item in Ash; mash Chrome should sync the change.

+ ash::ShelfItem item;

+ item.type = ash::TYPE_PINNED_APP;

+ item.id = ash::ShelfID(kDummyAppId);

+ if (is_mash)

+ shelf_controller->AddShelfItem(2, item);

+ else

+ model_.Add(item);

+ base::RunLoop().RunUntilIdle();

+ EXPECT_EQ(3, model_.item_count());

+ EXPECT_EQ(item.id, model_.items()[2].id);

+ EXPECT_EQ(is_mash ? 1u : 0u, shelf_controller->added_count());

+ EXPECT_EQ(0u, shelf_controller->removed_count());

+ // Simulate removing a shelf item in Ash; mash Chrome should sync the change.

+ if (is_mash)

+ shelf_controller->RemoveShelfItem(item.id);

+ else

+ model_.RemoveItemAt(2);

+ base::RunLoop().RunUntilIdle();

+ EXPECT_EQ(2, model_.item_count());

+ EXPECT_EQ(is_mash ? 1u : 0u, shelf_controller->added_count());

+ EXPECT_EQ(0u, shelf_controller->removed_count());

+ // Add an item in Chrome; Ash should be notified of the change in mash.

+ model_.Add(item);

+ EXPECT_EQ(3, model_.item_count());

+ base::RunLoop().RunUntilIdle();

+ EXPECT_EQ(is_mash ? 2u : 0u, shelf_controller->added_count());

+ EXPECT_EQ(0u, shelf_controller->removed_count());

+ // Remove an item in Chrome; Ash should be notified of the change in mash.

+ model_.RemoveItemAt(2);

+ EXPECT_EQ(2, model_.item_count());

+ base::RunLoop().RunUntilIdle();

+ EXPECT_EQ(is_mash ? 2u : 0u, shelf_controller->added_count());

+ EXPECT_EQ(is_mash ? 1u : 0u, shelf_controller->removed_count());