Implement the new Sync Confirmation dialog on Linux and Windows.

chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc

Index: chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc
diff --git a/chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc b/chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7bd4c7ccd5000b1d9d7b06013f2c3e06f274d6e9
--- /dev/null
+++ b/chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc
@@ -0,0 +1,223 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/webui/signin/sync_confirmation_handler.h"
+
+#include "base/values.h"
+#include "chrome/browser/profiles/profile_avatar_icon_util.h"
+#include "chrome/browser/signin/account_fetcher_service_factory.h"
+#include "chrome/browser/signin/account_tracker_service_factory.h"
+#include "chrome/browser/signin/fake_account_fetcher_service_builder.h"
+#include "chrome/browser/signin/fake_signin_manager_builder.h"
+#include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/webui/signin/sync_confirmation_ui.h"
+#include "chrome/test/base/browser_with_test_window_test.h"
+#include "chrome/test/base/dialog_test_browser_window.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/browser_sync/browser/profile_sync_service.h"
+#include "components/signin/core/browser/account_fetcher_service.h"
+#include "components/signin/core/browser/fake_account_fetcher_service.h"
+#include "components/signin/core/browser/fake_signin_manager.h"
+#include "content/public/test/test_web_ui.h"
+
+const int kExpectedProfileImageSize = 128;

[Dan Beam, 2016/01/26 19:54:20]

why are you copying this?

[anthonyvd, 2016/01/26 22:00:39]

It's the value I expect the image dimensions to have. I don't want it to be
public in the handler and I also want the test to break if it changes somewhere
else because that would break the layout of the dialog.

[Dan Beam, 2016/01/26 22:40:51]

i think we generally discourage change detection tests in chrome, but they also
exist, so whatevs

+
+class TestingSyncConfirmationHandler : public SyncConfirmationHandler {
+ public:
+  explicit TestingSyncConfirmationHandler(content::WebUI* web_ui) {
+    set_web_ui(web_ui);
+  }
+
+  void HandleConfirm(const base::ListValue* args) override {
+    SyncConfirmationHandler::HandleConfirm(args);
+  }
+
+  void HandleUndo(const base::ListValue* args) override {
+    SyncConfirmationHandler::HandleUndo(args);
+  }
+
+  void HandleInitialized(const base::ListValue* args) override {
+    SyncConfirmationHandler::HandleInitialized(args);
+  }
+
+  void HandleGoToSettings(const base::ListValue* args) override {
+    SyncConfirmationHandler::HandleGoToSettings(args);
+  }
+
+  void SetUserImageURL(const std::string& url) override {
+    SyncConfirmationHandler::SetUserImageURL(url);
+  }

[Dan Beam, 2016/01/26 19:54:20]

what are any of these doing?  is this just to increase visibility?  if so:

 public:
  using SyncConfirmationHandler::HandleConfirm;

should suffice

[anthonyvd, 2016/01/26 22:00:39]

Neat, didn't think about that. Thanks!

+};
+
+class SyncConfirmationHandlerTest : public BrowserWithTestWindowTest {

[Dan Beam, 2016/01/26 19:54:20]

this is actually a browser test (i.e. _unittest.cc -> _browsertest.cc,
unit_tests -> browser_tests)

[anthonyvd, 2016/01/26 22:00:39]

I think it's actually a BrowserTest. All subclasses of BrowserWithTestWindowTest
are in *_unittests.cc files and the inheritance chain doesn't go through
InProcessBrowserTest.

[Dan Beam, 2016/01/26 22:40:51]

i assume you mean "actually a testing::Test" or "Unit test" -- OK, I didn't know
this doesn't touch InProcessBrowserTests.  _unittest.cc is fine

+ public:
+  SyncConfirmationHandlerTest() : web_ui_(new content::TestWebUI) {}

[Dan Beam, 2016/01/26 19:54:20]

why do you need to heap allocate |web_ui_|?

[anthonyvd, 2016/01/26 22:00:39]

I need it to be destroyed in TearDown before
BrowserWithTestWindowTest::TearDown. Not having it there was an oversight, I
changed it to a scoped_ptr and called reset() in TearDown.

Thanks!

+
+  void SetUp() override {
+    BrowserWithTestWindowTest::SetUp();
+    chrome::NewTab(browser());
+    web_ui_->set_web_contents(
+        browser()->tab_strip_model()->GetActiveWebContents());
+    handler_.reset(new TestingSyncConfirmationHandler(web_ui_));
+    sync_confirmation_ui_.reset(
+        new SyncConfirmationUI(web_ui_, handler_.get()));
+
+    // This dialog assumes the signin flow was completed, which kicks off the
+    // SigninManager.
+    new OneClickSigninSyncStarter(

[Dan Beam, 2016/01/26 19:54:20]

how is this deleted?

[anthonyvd, 2016/01/26 22:00:39]

Instances of this class delete themselves when they're done.

+        profile(),
+        browser(),
+        "gaia",
+        "foo@example.com",
+        "password",
+        "refresh_token",
+        OneClickSigninSyncStarter::SYNC_WITH_DEFAULT_SETTINGS,
+        nullptr,
+        OneClickSigninSyncStarter::NO_CONFIRMATION,
+        GURL(),
+        GURL(),
+        OneClickSigninSyncStarter::Callback());
+  }
+
+  void TearDown() override {
+    handler_.reset();
+    sync_confirmation_ui_.reset();
+    signin_manager()->ForceSignOut();
+    BrowserWithTestWindowTest::TearDown();
+  }
+
+  TestingSyncConfirmationHandler* handler() {
+    return handler_.get();
+  }
+
+  content::TestWebUI* web_ui() {
+    return web_ui_;
+  }
+
+  FakeAccountFetcherService* account_fetcher_service() {
+    return static_cast<FakeAccountFetcherService*>(
+        AccountFetcherServiceFactory::GetForProfile(profile()));
+  }
+
+  FakeSigninManager* signin_manager() {
+    return static_cast<FakeSigninManager*>(
+        SigninManagerFactory::GetForProfile(profile()));
+  }
+
+  ProfileSyncService* sync() {
+    return ProfileSyncServiceFactory::GetForProfile(profile());
+  }
+
+  // BrowserWithTestWindowTest
+  BrowserWindow* CreateBrowserWindow() override {
+    return new DialogTestBrowserWindow;

[Dan Beam, 2016/01/26 19:54:20]

who owns this?

[anthonyvd, 2016/01/26 22:00:39]

The test class owns it and BrowserWithTestWindowTest::TearDown() takes care of
cleaning it up.

+  }
+
+  TestingProfile* CreateProfile() override {
+    TestingProfile::Builder builder;
+    builder.AddTestingFactory(AccountFetcherServiceFactory::GetInstance(),
+                              FakeAccountFetcherServiceBuilder::BuildForTests);
+    builder.AddTestingFactory(
+        SigninManagerFactory::GetInstance(), BuildFakeSigninManagerBase);
+    return builder.Build().release();
+  }
+
+private:
+  scoped_ptr<SyncConfirmationUI> sync_confirmation_ui_;
+  scoped_ptr<TestingSyncConfirmationHandler> handler_;
+  content::TestWebUI* web_ui_;
+};
+
+TEST_F(SyncConfirmationHandlerTest, TestSetImageIfPrimaryAccountReady) {
+  account_fetcher_service()->FakeUserInfoFetchSuccess(
+      "gaia",
+      "foo@example.com",
+      "gaia",
+      "",
+      "full_name",
+      "given_name",
+      "locale",
+      "http://picture.example.com/picture.jpg");
+
+  handler()->HandleInitialized(nullptr);
+  EXPECT_EQ(1UL, web_ui()->call_data().size());

[Dan Beam, 2016/01/26 19:54:20]

can you drop the L from all these if it's not necessary?

[anthonyvd, 2016/01/26 22:00:39]

Done.

+  EXPECT_EQ("sync.confirmation.setUserImageURL",
+            web_ui()->call_data()[0]->function_name());
+  EXPECT_TRUE(
+      web_ui()->call_data()[0]->arg1()->IsType(base::Value::TYPE_STRING));
+  std::string passed_picture_url;
+  EXPECT_TRUE(
+      web_ui()->call_data()[0]->arg1()->GetAsString(&passed_picture_url));
+
+  std::string original_picture_url =
+      AccountTrackerServiceFactory::GetForProfile(profile())->
+          GetAccountInfo("gaia").picture_url;
+  GURL picture_url_with_size;
+  EXPECT_TRUE(profiles::GetImageURLWithThumbnailSize(GURL(original_picture_url),
+                                                     kExpectedProfileImageSize,
+                                                     &picture_url_with_size));
+  EXPECT_EQ(picture_url_with_size.spec(), passed_picture_url);
+  handler()->HandleUndo(nullptr);
+}
+
+TEST_F(SyncConfirmationHandlerTest, TestSetImageIfPrimaryAccountReadyLater) {
+  handler()->HandleInitialized(nullptr);
+  EXPECT_EQ(0UL, web_ui()->call_data().size());
+
+  account_fetcher_service()->FakeUserInfoFetchSuccess(
+      "gaia",
+      "foo@example.com",
+      "gaia",
+      "",
+      "full_name",
+      "given_name",
+      "locale",
+      "http://picture.example.com/picture.jpg");
+
+  EXPECT_EQ(1UL, web_ui()->call_data().size());
+  EXPECT_EQ("sync.confirmation.setUserImageURL",
+            web_ui()->call_data()[0]->function_name());
+  EXPECT_TRUE(
+      web_ui()->call_data()[0]->arg1()->IsType(base::Value::TYPE_STRING));
+  std::string passed_picture_url;
+  EXPECT_TRUE(
+      web_ui()->call_data()[0]->arg1()->GetAsString(&passed_picture_url));
+
+  std::string original_picture_url =
+      AccountTrackerServiceFactory::GetForProfile(profile())->
+          GetAccountInfo("gaia").picture_url;
+  GURL picture_url_with_size;
+  EXPECT_TRUE(profiles::GetImageURLWithThumbnailSize(GURL(original_picture_url),
+                                                     kExpectedProfileImageSize,
+                                                     &picture_url_with_size));
+  EXPECT_EQ(picture_url_with_size.spec(), passed_picture_url);
+  handler()->HandleUndo(nullptr);
+}
+
+TEST_F(SyncConfirmationHandlerTest, TestHandleUndo) {
+  EXPECT_FALSE(sync()->IsFirstSetupComplete());
+  EXPECT_TRUE(sync()->IsFirstSetupInProgress());
+
+  handler()->HandleUndo(nullptr);
+
+  EXPECT_FALSE(sync()->IsFirstSetupInProgress());
+  EXPECT_FALSE(sync()->IsFirstSetupComplete());
+  EXPECT_FALSE(
+      SigninManagerFactory::GetForProfile(profile())->IsAuthenticated());
+}
+
+TEST_F(SyncConfirmationHandlerTest, TestHandleConfirm) {
+  EXPECT_FALSE(sync()->IsFirstSetupComplete());
+  EXPECT_TRUE(sync()->IsFirstSetupInProgress());
+
+  handler()->HandleConfirm(nullptr);
+
+  EXPECT_FALSE(sync()->IsFirstSetupInProgress());
+  EXPECT_TRUE(sync()->IsFirstSetupComplete());
+  EXPECT_TRUE(
+      SigninManagerFactory::GetForProfile(profile())->IsAuthenticated());
+}