Restrict CM API interface request and message dispatch.

chrome/browser/password_manager/chrome_password_manager_client.cc

 // Copyright 2014 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/password_manager/chrome_password_manager_client.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 355 matching lines @@
       possible_auto_sign_in_->origin == form.origin) {
     PromptUserToEnableAutosigninIfNecessary();
   }
   possible_auto_sign_in_.reset();
 }
 
 void ChromePasswordManagerClient::NotifyStorePasswordCalled() {
   // If a site stores a credential the autofill password manager shouldn't kick
   // in.
   password_manager_.DropFormManagers();
+  was_store_ever_called_ = true;
 }
 
 void ChromePasswordManagerClient::AutomaticPasswordSave(
     std::unique_ptr<password_manager::PasswordFormManager> saved_form) {
 #if defined(OS_ANDROID)
   GeneratedPasswordSavedInfoBarDelegateAndroid::Create(web_contents());
 #else
   PasswordsClientUIDelegate* manage_passwords_ui_controller =
       PasswordsClientUIDelegateFromWebContents(web_contents());
   manage_passwords_ui_controller->OnAutomaticPasswordSave(
@@ skipping 61 matching lines @@
   // web contents), once the UKM framework provides a mechanism for that.
   if (!ukm_source_id_) {
     ukm_source_id_ = ukm::UkmRecorder::GetNewSourceID();
     ukm::UkmRecorder* ukm_recorder = GetUkmRecorder();
     if (ukm_recorder)
       ukm_recorder->UpdateSourceURL(*ukm_source_id_, GetMainFrameURL());
   }
   return *ukm_source_id_;
 }
 
-// TODO(crbug.com/706392): Fix password reuse detection for Android.
-#if !defined(OS_ANDROID)
 void ChromePasswordManagerClient::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->IsInMainFrame() || !navigation_handle->HasCommitted())
     return;
 
   if (!navigation_handle->IsSameDocument())
     ukm_source_id_.reset();
 
+  // From this point on, the CredentialManagerImpl will service API calls in the
+  // context of the new WebContents::GetLastCommittedURL, which may very well be
+  // cross-origin. Disconnect existing client, and drop pending requests.
+  if (!navigation_handle->IsSameDocument())
+    credential_manager_impl_.DisconnectBinding();
+
+// TODO(crbug.com/706392): Fix password reuse detection for Android.
+#if !defined(OS_ANDROID)
   password_reuse_detection_manager_.DidNavigateMainFrame(GetMainFrameURL());
   // After some navigations RenderViewHost persists and just adding the observer
   // will cause multiple call of OnInputEvent. Since Widget API doesn't allow to
   // check whether the observer is already added, the observer is removed and
   // added again, to ensure that it is added only once.
   web_contents()->GetRenderViewHost()->GetWidget()->RemoveInputEventObserver(
       this);
   web_contents()->GetRenderViewHost()->GetWidget()->AddInputEventObserver(this);
+#endif
 }
 
+#if !defined(OS_ANDROID)
 void ChromePasswordManagerClient::OnInputEvent(
     const blink::WebInputEvent& event) {
   if (event.GetType() != blink::WebInputEvent::kChar)
     return;
   const blink::WebKeyboardEvent& key_event =
       static_cast<const blink::WebKeyboardEvent&>(event);
   password_reuse_detection_manager_.OnKeyPressed(key_event.text);
 }
 #endif
 
@@ skipping 220 matching lines @@
   return &credentials_filter_;
 }
 
 const password_manager::LogManager* ChromePasswordManagerClient::GetLogManager()
     const {
   return log_manager_.get();
 }
 
 // static
 void ChromePasswordManagerClient::BindCredentialManager(
-    const service_manager::BindSourceInfo& source_info,
-    password_manager::mojom::CredentialManagerRequest request,
+    password_manager::mojom::CredentialManagerAssociatedRequest request,
     content::RenderFrameHost* render_frame_host) {
   // Only valid for the main frame.
   if (render_frame_host->GetParent())
     return;
 
   content::WebContents* web_contents =
       content::WebContents::FromRenderFrameHost(render_frame_host);
   DCHECK(web_contents);
 
+  // Only valid for the currently committed RenderFrameHost, and not, e.g. old
+  // zombie RFH's being swapped out following cross-origin navigations.

[dcheng, 2017/07/03 09:23:33]

Hmm... I feel like something at a higher-level is broken if we violate this
condition... I don't think we should be forwarding bind requests for swapped out
frames at all.

[engedy, 2017/07/03 09:43:34]

Yes, I agree that this is a bit weird, but note the subtle distinction between
`swapped out` and `being swapped out`. Currently the ordering of events is as
follows:

 1) FrameHostMsg_DidCommitProvisionalLoad for provisional RFH
 2) RenderFrameHostManager::CommitPending()
 3) FrameMsg_SwapOut sent, RFH put on |pending_delete_hosts_|
 4) The `unload` handler invoked on old Document
 4.1) InterfaceRequest sent, potentially followed by method call
 5) FrameHostMsg_SwapOut_ACK sent
 6) RFH deleted

Obviously this is also orthogonal to using associated interfaces or not.

The browser tests with |preestablish_mojo_pipe == false| exercise this scenario,
and would fail without this check.

+  if (web_contents->GetMainFrame() != render_frame_host)
+    return;
+
   ChromePasswordManagerClient* instance =
       ChromePasswordManagerClient::FromWebContents(web_contents);
 
   // Try to bind to the driver, but if driver is not available for this render
   // frame host, the request will be just dropped. This will cause the message
   // pipe to be closed, which will raise a connection error on the peer side.
   if (!instance)
     return;
 
   instance->credential_manager_impl_.BindRequest(std::move(request));
 }
 
 // static
 bool ChromePasswordManagerClient::CanShowBubbleOnURL(const GURL& url) {
   std::string scheme = url.scheme();
   return (content::ChildProcessSecurityPolicy::GetInstance()->IsWebSafeScheme(
               scheme) &&
 #if BUILDFLAG(ENABLE_EXTENSIONS)
           scheme != extensions::kExtensionScheme &&
 #endif
           scheme != content::kChromeDevToolsScheme);
 }