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..941a29264bc1ddb2ab262abf88c28df0d79ed309 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,36 @@ 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) {}
+
+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) ==
-        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

[sky, 2016/08/02 03:14:08]

I think this would be less confusing if you cached the WebContents* for the top
and checked again it. Less error prone.

[Moe, 2016/08/02 15:22:59]

Done.
In fact since the |source| WebContents is passed to the function we don't need
to cache it at all. The logic can be simplified even further by looking for the
guest WebContents only if guest_web_contents_ hasn't already been set.

-  // webview and observe it instead.  The webview may not be found in the
+  // webview and manage that 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(chrome::kChromeUIChromeSigninURL))
     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();

[sky, 2016/07/25 14:53:22]

How come web_contents_ is reset here? This class then becomes the delegate for
multiple WebContents. How do you guarantee this class outlives all the
webcontents?

[Moe, 2016/08/01 21:44:59]

Sorry for the late reply...
Because we would need to be delegate for WebContents of its guest webview (see
new_inline_login.html) in order to handle its context menu action.
The host WebView is owned by ReauthDelegate which implements
BaseReauthDialogDelegate (cocoa's impl is similar) and outlives it. I believe
the guest WebView's life is also dependent on its host. So we shouldn't have any
problems here.

+    web_contents_->SetDelegate(this);
+  }
 }