Excluded non-chromeos source files in chromeos build

chrome/browser/ui/views/bookmarks/bookmark_bubble_sign_in_delegate_browsertest.cc

 // Copyright 2013 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/bookmarks/bookmark_bubble_sign_in_delegate.h"
 
 #include <memory>
 
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/test_extension_service.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/sync/bubble_sync_promo_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/browser/ui/views/profiles/profile_chooser_view.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/test_utils.h"
 #include "ui/events/event_constants.h"
 #include "ui/gfx/range/range.h"
 
-#if defined(OS_CHROMEOS)
-const bool kHasProfileChooser = false;
-#else
-const bool kHasProfileChooser = true;
+#if !defined(OS_CHROMEOS)
+#include "chrome/browser/ui/views/profiles/profile_chooser_view.h"
 #endif
 
 class BookmarkBubbleSignInDelegateTest : public InProcessBrowserTest {
  public:
   BookmarkBubbleSignInDelegateTest() {}
 
   Profile* profile() { return browser()->profile(); }
 
   void ReplaceBlank(Browser* browser);
 
+  void SignInBrowser(Browser* browser);
+  void CreateRemoveSigninBrowser(Browser* last_active);

[msw, 2016/07/19 18:51:04]

This name is a bit confusing, and it seems like you could easily leave the
implementation inlined within the BrowserRemoved fixture.

[Jane, 2016/07/19 21:29:56]

Done. Removed this and put the code back.

+
  private:
   DISALLOW_COPY_AND_ASSIGN(BookmarkBubbleSignInDelegateTest);
 };
 
 // The default browser created for tests start with one tab open on
 // about:blank.  The sign-in page is a singleton that will
 // replace this tab.  This function replaces about:blank with another URL
 // so that the sign in page goes into a new tab.
 void BookmarkBubbleSignInDelegateTest::ReplaceBlank(Browser* browser) {
   chrome::NavigateParams params(
       chrome::GetSingletonTabNavigateParams(browser, GURL("chrome:version")));
   params.path_behavior = chrome::NavigateParams::IGNORE_AND_NAVIGATE;
   chrome::ShowSingletonTabOverwritingNTP(browser, params);
 }
 
+void BookmarkBubbleSignInDelegateTest::SignInBrowser(Browser* browser) {
+  std::unique_ptr<BubbleSyncPromoDelegate> delegate;
+  delegate.reset(new BookmarkBubbleSignInDelegate(browser));
+  delegate->OnSignInLinkClicked();
+}
+
+void BookmarkBubbleSignInDelegateTest::CreateRemoveSigninBrowser(
+    Browser* last_active) {
+  std::unique_ptr<BubbleSyncPromoDelegate> delegate;
+  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));
+
+  BrowserList::SetLastActive(last_active);
+
+  // Close all tabs in the original browser. Run all pending messages to make
+  // sure the browser window closes before continuing.
+  browser()->tab_strip_model()->CloseAllTabs();
+  content::RunAllPendingInMessageLoop();
+
+  delegate->OnSignInLinkClicked();
+}
+
 IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, OnSignInLinkClicked) {
   ReplaceBlank(browser());
   int starting_tab_count = browser()->tab_strip_model()->count();
+  SignInBrowser(browser());
 
-  std::unique_ptr<BubbleSyncPromoDelegate> delegate;
-  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));
+#if !defined(OS_CHROMEOS)
+  if (switches::UsePasswordSeparatedSigninFlow()) {

[msw, 2016/07/19 18:51:04]

optional nit: drop all these unnecessary curly braces.

[Jane, 2016/07/19 21:29:56]

Done.

+    EXPECT_TRUE(browser()->signin_view_controller()->delegate());
+  } else {
+    EXPECT_TRUE(ProfileChooserView::IsShowing());
+  }
 
-  delegate->OnSignInLinkClicked();
-
-  if (kHasProfileChooser) {
-    if (switches::UsePasswordSeparatedSigninFlow()) {
-      EXPECT_TRUE(browser()->signin_view_controller()->delegate());
-    } else {
-      EXPECT_TRUE(ProfileChooserView::IsShowing());
-    }
-
-    EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
-  } else {
-    EXPECT_EQ(starting_tab_count + 1, browser()->tab_strip_model()->count());
-  }
+  EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
+#else
+  EXPECT_EQ(starting_tab_count + 1, browser()->tab_strip_model()->count());
+#endif
 }
 
 IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest,
                        OnSignInLinkClickedReusesBlank) {
   int starting_tab_count = browser()->tab_strip_model()->count();
+  SignInBrowser(browser());
 
-  std::unique_ptr<BubbleSyncPromoDelegate> delegate;
-  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));
+#if !defined(OS_CHROMEOS)
+  if (switches::UsePasswordSeparatedSigninFlow()) {
+    EXPECT_TRUE(browser()->signin_view_controller()->delegate());
+  } else {
+    EXPECT_TRUE(ProfileChooserView::IsShowing());
+  }
 
-  delegate->OnSignInLinkClicked();
-
-  if (kHasProfileChooser) {
-    if (switches::UsePasswordSeparatedSigninFlow()) {
-      EXPECT_TRUE(browser()->signin_view_controller()->delegate());
-    } else {
-      EXPECT_TRUE(ProfileChooserView::IsShowing());
-    }
-
-    EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
-  } else {
-    EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
-  }
+  EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
+#else
+  EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
+#endif
 }
 
 IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest,
                        OnSignInLinkClickedIncognito) {
   ReplaceBlank(browser());
   Browser* incognito_browser = CreateIncognitoBrowser();
-
-  int starting_tab_count_normal = browser()->tab_strip_model()->count();
   int starting_tab_count_incognito =
       incognito_browser->tab_strip_model()->count();
 
-  std::unique_ptr<BubbleSyncPromoDelegate> delegate;
-  delegate.reset(new BookmarkBubbleSignInDelegate(incognito_browser));
+#if !defined(OS_CHROMEOS)
+  SignInBrowser(incognito_browser);
+  // ProfileChooser doesn't show in an incognito window.
+  EXPECT_FALSE(ProfileChooserView::IsShowing());
+#else
+  int starting_tab_count_normal = browser()->tab_strip_model()->count();

[msw, 2016/07/19 18:51:04]

Leave this above, with the call to SignInBrowser, and the |tab_count_normal|
declaration, then just test they're equal in the non-cros case:

  int starting_tab_count_normal = browser()->tab_strip_model()->count();
  int starting_tab_count_incognito =
      incognito_browser->tab_strip_model()->count();

  SignInBrowser(incognito_browser);

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

#if !defined(OS_CHROMEOS)
  // ProfileChooser doesn't show in an incognito window.
  EXPECT_FALSE(ProfileChooserView::IsShowing());
  // No new tab should have been opened in the normal browser.
  EXPECT_EQ(starting_tab_count_normal, tab_count_normal);
#else
  // A new tab should have been opened in the normal browser.
  EXPECT_EQ(starting_tab_count_normal + 1, tab_count_normal);
#endif

[Jane, 2016/07/19 21:29:56]

Done.

+  SignInBrowser(incognito_browser);
 
-  delegate->OnSignInLinkClicked();
-
-  if (kHasProfileChooser) {
-    // ProfileChooser doesn't show in an incognito window.
-    EXPECT_FALSE(ProfileChooserView::IsShowing());
-  } else {
-    // A new tab should have been opened in the normal browser, which should be
-    // visible.
-    int tab_count_normal = browser()->tab_strip_model()->count();
-    EXPECT_EQ(starting_tab_count_normal + 1, tab_count_normal);
-  }
+  // A new tab should have been opened in the normal browser, which should be
+  // visible.
+  int tab_count_normal = browser()->tab_strip_model()->count();
+  EXPECT_EQ(starting_tab_count_normal + 1, tab_count_normal);
+#endif
   // No effect is expected on the incognito browser.
   int tab_count_incognito = incognito_browser->tab_strip_model()->count();
   EXPECT_EQ(starting_tab_count_incognito, tab_count_incognito);
 }
 
 // Verifies that the sign in page can be loaded in a different browser
 // if the provided browser is invalidated.
 IN_PROC_BROWSER_TEST_F(BookmarkBubbleSignInDelegateTest, BrowserRemoved) {
   // Create an extra browser.
   Browser* extra_browser = CreateBrowser(profile());
   ReplaceBlank(extra_browser);
 
+#if !defined(OS_CHROMEOS)
+  CreateRemoveSigninBrowser(extra_browser);
+  if (switches::UsePasswordSeparatedSigninFlow()) {
+    EXPECT_TRUE(extra_browser->signin_view_controller()->delegate());
+  } else {
+    EXPECT_TRUE(ProfileChooserView::IsShowing());
+  }
+#else
   int starting_tab_count = extra_browser->tab_strip_model()->count();

[msw, 2016/07/19 18:51:04]

Try to avoid some of this refactoring. You should be able to just replace the
|if (kHasProfileChooser)| condition with the |#if !defined(OS_CHROMEOS)|
preprocessor block; and test that |starting_tab_count| hasn't changed in the
OS_CHROMEOS case (or only define it in a separate |#if defined(OS_CHROMEOS)|
preprocessor block above); consider:

  int starting_tab_count = extra_browser->tab_strip_model()->count();

  std::unique_ptr<BubbleSyncPromoDelegate> delegate;
  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));

  BrowserList::SetLastActive(extra_browser);

  // Close all tabs in the original browser.  Run all pending messages	
  // to make sure the browser window closes before continuing.
  browser()->tab_strip_model()->CloseAllTabs();
  content::RunAllPendingInMessageLoop();

  delegate->OnSignInLinkClicked();

  int tab_count = extra_browser->tab_strip_model()->count();
#if !defined(OS_CHROMEOS)
  if (switches::UsePasswordSeparatedSigninFlow())
    EXPECT_TRUE(extra_browser->signin_view_controller()->delegate());
  else
    EXPECT_TRUE(ProfileChooserView::IsShowing());
  // No new tab should have been opened in the extra browser.
  EXPECT_EQ(starting_tab_count, tab_count);
#else
  // A new tab should have been opened in the extra browser.
  EXPECT_EQ(starting_tab_count + 1, tab_count);
#endif

[Jane, 2016/07/19 21:29:56]

Done. I did that much refactoring in an attempt to make all the ifdefs look
cleaner, but your way definitely looks better. Thanks for the suggestion!

-
-  std::unique_ptr<BubbleSyncPromoDelegate> delegate;
-  delegate.reset(new BookmarkBubbleSignInDelegate(browser()));
-
-  BrowserList::SetLastActive(extra_browser);
-
-  // Close all tabs in the original browser.  Run all pending messages
-  // to make sure the browser window closes before continuing.
-  browser()->tab_strip_model()->CloseAllTabs();
-  content::RunAllPendingInMessageLoop();
-
-  delegate->OnSignInLinkClicked();
-
-  if (kHasProfileChooser) {
-    if (switches::UsePasswordSeparatedSigninFlow()) {
-      EXPECT_TRUE(extra_browser->signin_view_controller()->delegate());
-    } else {
-      EXPECT_TRUE(ProfileChooserView::IsShowing());
-    }
-  } else {
-    // A new tab should have been opened in the extra browser, which should be
-    // visible.
-    int tab_count = extra_browser->tab_strip_model()->count();
-    EXPECT_EQ(starting_tab_count + 1, tab_count);
-  }
+  CreateRemoveSigninBrowser(extra_browser);
+  // A new tab should have been opened in the extra browser, which should be
+  // visible.
+  int tab_count = extra_browser->tab_strip_model()->count();
+  EXPECT_EQ(starting_tab_count + 1, tab_count);
+#endif
 }