Show non-incognito browser in front of incognito browser when signing in from bookmark bubble.

When sign in is initiated from the bookmark bubble in an incognito
browser, the sign in page is not displayed in the browser provided to
chrome::ShowBrowserSignin() but in a new/existing non-incognito browser.
BookmarkBubbleSignInDelegate was not aware of that and called Show()
on the wrong browser.

This call was unnecessary because chrome::ShowBrowserSignin() already
make it on the right browser. I removed the unnecessary call.

BUG=282305
TEST=
Pre-condition: Make sure you are not signed in to Chrome.
1.Launch Chrome, open 'Incognito window' from hot-dog menu and navigate to www.google.com.
2.Click on bookmark icon(star) in omnibox and click on 'Sign in' link of bookmark dialog box.
Expected result: Sign in page is displayed in a non-incognito browser.
This browser is displayed in front of the incognito browser.
R=rogerta

Review URL: https://chromiumcodereview.appspot.com/23726016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221212 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate.cc

@@ -26,8 +26,6 @@ BookmarkBubbleSignInDelegate::~BookmarkBubbleSignInDelegate() {
 void BookmarkBubbleSignInDelegate::OnSignInLinkClicked() {
   EnsureBrowser();
   chrome::ShowBrowserSignin(browser_, signin::SOURCE_BOOKMARK_BUBBLE);
-  DCHECK(!browser_->tab_strip_model()->empty());
-  browser_->window()->Show();
 }
 
 void BookmarkBubbleSignInDelegate::OnBrowserRemoved(Browser* browser) {

chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_unittest.cc

@@ -4,16 +4,47 @@
 
 #include "chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate.h"
 
+#include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/ui/bookmarks/bookmark_bubble_delegate.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
+#include "chrome/test/base/testing_profile.h"
 #include "ui/base/events/event_constants.h"
 #include "ui/base/range/range.h"
 
-typedef BrowserWithTestWindowTest BookmarkBubbleSignInDelegateTest;
+class BookmarkBubbleSignInDelegateTest : public BrowserWithTestWindowTest {
+ public:
+  BookmarkBubbleSignInDelegateTest() {}
+
+ protected:
+  class Window : public TestBrowserWindow {
+   public:
+    Window() : show_count_(0) {}
+
+    int show_count() { return show_count_; }
+
+   private:
+    // TestBrowserWindow:
+    virtual void Show() OVERRIDE {
+      ++show_count_;
+    }
+
+    // Number of times that the Show() method has been called.
+    int show_count_;
+
+    DISALLOW_COPY_AND_ASSIGN(Window);
+  };
+
+  virtual BrowserWindow* CreateBrowserWindow() OVERRIDE {
+    return new Window();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BookmarkBubbleSignInDelegateTest);
+};
 
 TEST_F(BookmarkBubbleSignInDelegateTest, OnSignInLinkClicked) {
   int starting_tab_count = browser()->tab_strip_model()->count();
@@ -23,9 +54,54 @@ TEST_F(BookmarkBubbleSignInDelegateTest, OnSignInLinkClicked) {
 
   delegate->OnSignInLinkClicked();
 
-  // A new tab should have been opened.
+  // A new tab should have been opened and the browser should be visible.
   int tab_count = browser()->tab_strip_model()->count();
   EXPECT_EQ(starting_tab_count + 1, tab_count);
+  EXPECT_EQ(1,
+            static_cast<BookmarkBubbleSignInDelegateTest::Window*>(
+                browser()->window())->show_count());
+}
+
+TEST_F(BookmarkBubbleSignInDelegateTest, OnSignInLinkClickedIncognito) {
+  // Create an incognito browser.
+  TestingProfile::Builder incognito_profile_builder;
+  incognito_profile_builder.SetIncognito();
+  scoped_ptr<TestingProfile> incognito_profile =
+      incognito_profile_builder.Build();
+  incognito_profile->SetOriginalProfile(profile());
+  profile()->SetOffTheRecordProfile(incognito_profile.PassAs<Profile>());
+
+  scoped_ptr<BrowserWindow> incognito_window;
+  incognito_window.reset(CreateBrowserWindow());
+  Browser::CreateParams params(browser()->profile()->GetOffTheRecordProfile(),
+                               browser()->host_desktop_type());
+  params.window = incognito_window.get();
+  scoped_ptr<Browser> incognito_browser;
+  incognito_browser.reset(new Browser(params));
+
+  int starting_tab_count_normal = browser()->tab_strip_model()->count();
+  int starting_tab_count_incognito =
+      incognito_browser.get()->tab_strip_model()->count();
+
+  scoped_ptr<BookmarkBubbleDelegate> delegate;
+  delegate.reset(new BookmarkBubbleSignInDelegate(incognito_browser.get()));
+
+  delegate->OnSignInLinkClicked();
+
+  // 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);
+  EXPECT_EQ(1,
+            static_cast<BookmarkBubbleSignInDelegateTest::Window*>(
+                browser()->window())->show_count());
+
+  // 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);
+  EXPECT_EQ(0,
+            static_cast<BookmarkBubbleSignInDelegateTest::Window*>(
+                incognito_window.get())->show_count());
 }
 
 // Verifies that the sign in page can be loaded in a different browser
@@ -53,9 +129,13 @@ TEST_F(BookmarkBubbleSignInDelegateTest, BrowserRemoved) {
 
   delegate->OnSignInLinkClicked();
 
-  // A new tab should have been opened in the 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);
+  EXPECT_EQ(1,
+            static_cast<BookmarkBubbleSignInDelegateTest::Window*>(
+                extra_window.get())->show_count());
 
   // Required to avoid a crash when the browser is deleted.
   extra_browser->tab_strip_model()->CloseAllTabs();