Fix unwanted sign in to Chrome when the user signs in to another service.

chrome/browser/ui/sync/one_click_signin_helper.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/sync/one_click_signin_helper.h"
 
 #include <algorithm>
 #include <functional>
 #include <utility>
 #include <vector>
@@ skipping 1034 matching lines @@
       url.ReplaceComponents(replacements) !=
           continue_url_.ReplaceComponents(replacements)) {
     if (++untrusted_navigations_since_signin_visit_ > kMaxNavigationsSince)
       CleanTransientState();
   }
 }
 
 void OneClickSigninHelper::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
-  // If we navigate to a non-sign-in URL, make sure that the renderer process
-  // is no longer considered the trusted sign-in process.
   if (!SigninManager::IsWebBasedSigninFlowURL(params.url)) {
+    // Make sure the renderer process is no longer considered the trusted
+    // sign-in process when a navigation to a non-sign-in URL occurs.
     Profile* profile =
         Profile::FromBrowserContext(web_contents()->GetBrowserContext());
     SigninManager* manager = profile ?
         SigninManagerFactory::GetForProfile(profile) : NULL;
     int process_id = web_contents()->GetRenderProcessHost()->GetID();
     if (manager && manager->IsSigninProcess(process_id))
       manager->ClearSigninProcess();
+
+    // If the navigation to a non-sign-in URL hasn't been triggered by the web
+    // contents, the sign in flow has been aborted and the state must be
+    // cleaned (crbug.com/269421).
+    if (!content::PageTransitionIsWebTriggerable(params.transition) &&
+        auto_accept_ != AUTO_ACCEPT_NONE)

[fdoray, 2013/08/09 14:32:25]

CleanTransientState() cannot be executed in a unit test because it posts a task
to the IO thread. I added "auto_accept_ != AUTO_ACCEPT_NONE" so that it's not
executed when it's not necessary.

[(NOT FOR CODE REVIEWS), 2013/08/09 14:43:04]

Is it only for tests?  Can you set do_not_clear_pending_email_ to true?

[fdoray, 2013/08/09 17:42:14]

It's for tests, but it also avoids unnecessary calls to CleanTransientState().

It would be hard to set do_not_clear_pending_email_ for every
OneClickSigninHelper created in a BrowserWithTestWindowTest (see unit_tests 
http://build.chromium.org/p/tryserver.chromium/builders/linux_rel/builds/157212).

I don't think that the variables cleaned by CleanTransientState() can have a
value when auto_accept_ == AUTO_ACCEPT_NONE. Am I right? If yes, can I move the
if (auto_accept_ != AUTO_ACCEPT_NONE) condition in CleanTransientState()?

On 2013/08/09 14:43:04, Roger Tawa (Google) wrote:

+      CleanTransientState();

[(NOT FOR CODE REVIEWS), 2013/08/09 14:43:04]

Should add { and } to this if statement since condition is across two lines.

[fdoray, 2013/08/09 17:42:14]

Done.

   }
 }
 
 void OneClickSigninHelper::DidStopLoading(
     content::RenderViewHost* render_view_host) {
   // If the user left the sign in process, clear all members.
   // TODO(rogerta): might need to allow some youtube URLs.
   content::WebContents* contents = web_contents();
   const GURL url = contents->GetLastCommittedURL();
   Profile* profile =
@@ skipping 297 matching lines @@
     // If the web contents is showing a blank page and not about to be closed,
     // redirect to the NTP or apps page.
     if (signin::IsContinueUrlForWebBasedSigninFlow(current_url) &&
         !signin::IsAutoCloseEnabledInURL(original_continue_url_)) {
       RedirectToNtpOrAppsPage(
           web_contents(),
           signin::GetSourceForPromoURL(original_continue_url_));
     }
   }
 }