Display local signin error without browser and record the path of selected profile in user manager.

chrome/browser/ui/views/profiles/user_manager_view.cc

Index: chrome/browser/ui/views/profiles/user_manager_view.cc
diff --git a/chrome/browser/ui/views/profiles/user_manager_view.cc b/chrome/browser/ui/views/profiles/user_manager_view.cc
index f7c9a67a89f56a5eae7e2170dcda4b33d7b774b1..149ebf49e4d390ae5115c8f96d7bc1bb27fed50c 100644
--- a/chrome/browser/ui/views/profiles/user_manager_view.cc
+++ b/chrome/browser/ui/views/profiles/user_manager_view.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/user_manager.h"
+#include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/guest_view/browser/guest_view_manager.h"
@@ -66,7 +67,7 @@ ReauthDelegate::ReauthDelegate(UserManagerView* parent,
   AddChildView(web_view_);
   SetLayoutManager(new views::FillLayout());
 
-  web_view->GetWebContents()->SetDelegate(this);
+  web_view_->GetWebContents()->SetDelegate(this);
 
   // Load the re-auth URL, prepopulated with the user's email address.
   // Add the index of the profile to the URL so that the inline login page
@@ -87,6 +88,10 @@ gfx::Size ReauthDelegate::GetPreferredSize() const {
                 UserManager::kPasswordCombinedReauthDialogHeight);
 }
 
+void ReauthDelegate::DisplayErrorMessage() {
+  web_view_->LoadInitialURL(GURL(chrome::kChromeUISigninErrorURL));
+}
+
 bool ReauthDelegate::CanResize() const {
   return true;
 }
@@ -210,17 +215,14 @@ void UserManager::ShowReauthDialog(content::BrowserContext* browser_context,
   // This method should only be called if the user manager is already showing.
   if (!IsShowing())
     return;
-
   instance_->ShowReauthDialog(browser_context, email, reason);
 }
 
 // static
 void UserManager::HideReauthDialog() {
   // This method should only be called if the user manager is already showing.
-  if (!IsShowing())
-    return;
-
-  instance_->HideReauthDialog();
+  if (instance_ && !instance_->GetWidget()->IsClosed())

[sky, 2016/09/20 23:30:19]

Can you motivate why this change is needed?

[zmin, 2016/09/21 00:09:35]

IsShowing will check the activate of User Manager window.
bool UserManager::IsShowing() {
 return instance_ ? instance_->GetWidget()->IsActive() : false;	 194   return
instance_ ? instance_->GetWidget()->IsActive() : false; }}

However, it's not true when gaia window(ReauthDialog) popup. Because when gaia
window(ReauthDialog) displayed, the user manager window will behind the gaia
window(ReauthDialog) and not activate and I can't call this function to hide the
dialog.

[sky, 2016/09/21 23:00:02]

I think you are arguing for checking IsVisible(), which makes sense.

+    instance_->HideReauthDialog();
 }
 
 // static
@@ -230,12 +232,33 @@ void UserManager::AddOnUserManagerShownCallbackForTesting(
   user_manager_shown_callback_for_testing_ = new base::Closure(callback);
 }
 
+// static
+void UserManager::ShowSigninDialog(content::BrowserContext* browser_context,
+                                   const base::FilePath profile_path) {
+  instance_->SetSigninProfilePath(profile_path);

[sky, 2016/09/20 23:30:19]

How do you know instance_ is non-null at the time this is called?

[zmin, 2016/09/21 00:09:35]

Good catch, I shall add a check here.

[zmin, 2016/09/21 16:40:04]

Done.

+  ShowReauthDialog(browser_context, std::string(),

[sky, 2016/09/20 23:30:19]

As someone not intimitely familiar with this code I find it confusing that
ShowSigninDialog calls ShowReauthDialog. Is there a reason we can't have a
single ShowReauthDialog with the appropriate details?

[zmin, 2016/09/21 00:09:35]

User manager only has ShowReauthDialog before because all sign in in User
Manager are reauth. However, it's not true anymore with force sign in which is
the reason I create ShowSigninDialog. But I call ShowReauthDialog inside the
ShowSigninDialog to avoid code copy.

[zmin, 2016/09/21 02:56:08]

I think it's more clear for the API or it will become "It's not reauth but fresh
sign in, why call UserManager::ShowReauthDialog()?" Another solution is change
the name of ShowReauthDialog but that is not a simply refactor.

+                   signin_metrics::Reason::REASON_SIGNIN_PRIMARY_ACCOUNT);
+}
+
+// static
+void UserManager::DisplayErrorMessage() {
+  // This method should only be called if the user manager is already showing.
+  if (instance_)
+    instance_->DisplayErrorMessage();
+}
+
+// static
+base::FilePath UserManager::GetSigninProfilePath() {
+  return instance_->GetSigninProfilePath();
+}
+
 // UserManagerView -------------------------------------------------------------
 
 UserManagerView::UserManagerView()
     : web_view_(nullptr),
       delegate_(nullptr),
-      user_manager_started_showing_(base::Time()) {
+      user_manager_started_showing_(base::Time()),
+      signin_profile_path_(base::FilePath()) {

[sky, 2016/09/20 23:30:19]

This is the default, so you don't need it.

[zmin, 2016/09/21 16:40:04]

Done.

   keep_alive_.reset(new ScopedKeepAlive(KeepAliveOrigin::USER_MANAGER_VIEW,
                                         KeepAliveRestartOption::DISABLED));
 }
@@ -281,6 +304,12 @@ void UserManagerView::HideReauthDialog() {
   }
 }
 
+void UserManagerView::DisplayErrorMessage() {

[sky, 2016/09/20 23:30:19]

Style guide says declaration and definition order should match.

[zmin, 2016/09/21 16:40:04]

Done.

+  if (delegate_) {

[sky, 2016/09/20 23:30:19]

no {}

[zmin, 2016/09/21 16:40:04]

Done.

+    delegate_->DisplayErrorMessage();
+  }
+}
+
 void UserManagerView::OnReauthDialogDestroyed() {
   delegate_ = nullptr;
 }
@@ -408,3 +437,11 @@ void UserManagerView::WindowClosing() {
 bool UserManagerView::ShouldUseCustomFrame() const {
   return false;
 }
+
+void UserManagerView::SetSigninProfilePath(const base::FilePath profile_path) {
+  signin_profile_path_ = profile_path;
+}
+
+base::FilePath UserManagerView::GetSigninProfilePath() {
+  return signin_profile_path_;
+}