Disables the context menu in User Manager's reauth dialog

chrome/browser/ui/user_manager.cc

Index: chrome/browser/ui/user_manager.cc
diff --git a/chrome/browser/ui/user_manager.cc b/chrome/browser/ui/user_manager.cc
index 9426c81d64e01ebffce2756e74a102999e78d600..f22751d4c3fa815007f976a092af434f614ad4f4 100644
--- a/chrome/browser/ui/user_manager.cc
+++ b/chrome/browser/ui/user_manager.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/ui/user_manager.h"
 
 #include "chrome/browser/signin/signin_promo.h"
+#include "chrome/common/url_constants.h"
 #include "components/guest_view/browser/guest_view_manager.h"
 #include "google_apis/gaia/gaia_urls.h"
 
@@ -18,46 +19,37 @@ bool AddToSet(std::set<content::WebContents*>* content_set,
 
 }  // namespace
 
-UserManager::ReauthDialogObserver::ReauthDialogObserver(
-    content::WebContents* web_contents, const std::string& email_address)
-    : email_address_(email_address) {
-  // Observe navigations of the web contents so that the dialog can close itself
-  // when the sign in process is done.
-  Observe(web_contents);
+UserManager::BaseReauthDialogDelegate::BaseReauthDialogDelegate(
+      content::WebContents* web_contents) : web_contents_(web_contents) {}

[sky, 2016/07/22 22:29:01]

Why do you set the delegate on line 53 instead of here?

[Moe, 2016/07/22 23:56:56]

The delegate is set similarly in the cocoa logic. My thinking was that setting
the webContents' delegate feels more natural if done explicitly and setting it
here would be more appropriate for an observer. But no good reason really.

+
+bool UserManager::BaseReauthDialogDelegate::HandleContextMenu(
+    const content::ContextMenuParams& params) {
+  // Ignores context menu.
+  return true;
 }
 
-void UserManager::ReauthDialogObserver::DidStopLoading() {
-  // If the sign in process reaches the termination URL, close the dialog.
-  // Make sure to remove any parts of the URL that gaia might append during
-  // signin.
-  GURL url = web_contents()->GetURL();
-  url::Replacements<char> replacements;
-  replacements.ClearQuery();
-  replacements.ClearRef();
-  if (url.ReplaceComponents(replacements) ==

[sky, 2016/07/22 22:29:01]

How come you're changing this code? The description mentions menu, not this.

[Moe, 2016/07/22 23:56:56]

I mentioned in a comment in the initial patch that: "The reauth dialog is closed
explicitly in
InlineLoginHandler::HandleDialogClose(). This is not needed."
My bad. should've mentioned it in the desc.

-        GaiaUrls::GetInstance()->signin_completed_continue_url()) {
-    CloseReauthDialog();
+
+void UserManager::BaseReauthDialogDelegate::LoadingStateChanged(
+    content::WebContents* source, bool to_different_document) {
+  if (source->IsLoading())
     return;
-  }
 
   // If still observing the top level web contents, try to find the embedded
   // webview and observe it instead.  The webview may not be found in the
   // initial page load since it loads asynchronously.
-  if (url.GetOrigin() !=
-      signin::GetReauthURLWithEmail(
-          signin_metrics::AccessPoint::ACCESS_POINT_USER_MANAGER,
-          signin_metrics::Reason::REASON_UNLOCK, email_address_)
-          .GetOrigin()) {
+  GURL url = web_contents_->GetURL();
+  if (url.GetOrigin() != GURL(std::string(chrome::kChromeUIChromeSigninURL)))

[sky, 2016/07/22 22:29:01]

Do you really need the std::string here?

[Moe, 2016/07/22 23:56:56]

Not really. Removed.

     return;
-  }
 
   std::set<content::WebContents*> content_set;
   guest_view::GuestViewManager* manager =
       guest_view::GuestViewManager::FromBrowserContext(
-          web_contents()->GetBrowserContext());
+          web_contents_->GetBrowserContext());
   if (manager)
-    manager->ForEachGuest(web_contents(), base::Bind(&AddToSet, &content_set));
+    manager->ForEachGuest(web_contents_, base::Bind(&AddToSet, &content_set));
   DCHECK_LE(content_set.size(), 1U);
-  if (!content_set.empty())
-    Observe(*content_set.begin());
+  if (!content_set.empty()) {
+    web_contents_ = *content_set.begin();
+    web_contents_->SetDelegate(this);
+  }
 }