Add a method to LoginUIService to open the sign in UI in a popup.

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

Index: chrome/browser/ui/webui/signin/login_ui_service.cc
===================================================================
--- chrome/browser/ui/webui/signin/login_ui_service.cc	(revision 144917)
+++ chrome/browser/ui/webui/signin/login_ui_service.cc	(working copy)
@@ -5,9 +5,20 @@
 #include "chrome/browser/ui/webui/signin/login_ui_service.h"
 
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/browser/page_navigator.h"
+#include "content/public/common/page_transition_types.h"
+#include "content/public/common/referrer.h"
+#include "webkit/glue/window_open_disposition.h"
 
+using content::OpenURLParams;
+using content::Referrer;
+
 LoginUIService::LoginUIService() : ui_(NULL) {
 }
 
@@ -16,11 +27,15 @@
 void LoginUIService::SetLoginUI(LoginUI* ui) {
   DCHECK(!current_login_ui() || current_login_ui() == ui);
   ui_ = ui;
+  FireLoginUIChanged();
 }
 
 void LoginUIService::LoginUIClosed(LoginUI* ui) {
-  if (current_login_ui() == ui)
-    ui_ = NULL;
+  if (current_login_ui() != ui)
+    return;
+
+  ui_ = NULL;
+  FireLoginUIChanged();
 }
 
 void LoginUIService::ShowLoginUI(Browser* browser) {
@@ -33,3 +48,23 @@
   // Need to navigate to the settings page and display the UI.
   chrome::ShowSettingsSubPage(browser, chrome::kSyncSetupSubPage);
 }
+
+void LoginUIService::ShowLoginDialogUI(Browser* browser) {

[Andrew T Wilson (Slow), 2012/07/03 17:34:09]

This isn't displaying a dialog - it's displaying a popup.

Does it make sense to just allow the caller to pass in the type of
WindowOpenDisposition, for added flexibility?

Also, I'm not at all sure that this is so generally useful that we need to make
a common API for it. If it's only going to get called from one place, can we
move this functionality into the caller instead?

[Munjal (Google), 2012/07/03 17:52:17]

I changed the name to ShowLoginUIPopup. I think it is better to not let the
calelrs pass the windowOpenDisposition yet.

I don't mind moving it to the caller. But I think this is a better place since
the caller has to do the same stuff about focusing existing UI if present etc.
and for that it integrates with LoginUIService anyway.

[Andrew T Wilson (Slow), 2012/07/09 19:52:06]

Let's put this in the caller (take it out of this patch). In fact, given that
ShowLoginUI() is so simple now (all the logic has been moved out to
chrome::ShowSettingsSubPage()) I'm planning to remove that UI too.

[Munjal (Google), 2012/07/09 21:13:26]

Done.

+  if (ui_) {
+    // We already have active login UI - make it visible.
+    ui_->FocusUI();
+    return;
+  }
+
+  OpenURLParams params(
+      GURL(chrome::kChromeUISyncPromoURL), Referrer(), NEW_POPUP,
+      content::PAGE_TRANSITION_START_PAGE, false);
+  browser->OpenURL(params);
+}
+
+void LoginUIService::FireLoginUIChanged() const {
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_LOGIN_UI_CHANGED,
+      content::Source<LoginUIService>(this),
+      content::NotificationService::NoDetails());

[Andrew T Wilson (Slow), 2012/07/03 17:34:09]

Does it make sense to send out the current login UI with this notification?

[Munjal (Google), 2012/07/03 17:52:17]

We discussed that remember? In fact, your suggestion was to let hte callers
query LoginUIService for current login UI. I liked it :).

[Andrew T Wilson (Slow), 2012/07/09 19:52:06]

Yeah, I remember, and I was just reconsidering the question. I don't have a
strong opinion so I'm OK with whatever you decide.

+}