Restrict CM API interface request and message dispatch.

components/password_manager/content/browser/credential_manager_impl.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 "components/password_manager/content/browser/credential_manager_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
@@ skipping 22 matching lines @@
                         const CredentialInfo& info) {
   std::move(callback).Run(mojom::CredentialManagerError::SUCCESS, info);
 }
 
 }  // namespace
 
 // CredentialManagerImpl -------------------------------------------------
 
 CredentialManagerImpl::CredentialManagerImpl(content::WebContents* web_contents,
                                              PasswordManagerClient* client)
-    : WebContentsObserver(web_contents), client_(client), weak_factory_(this) {
+    : WebContentsObserver(web_contents),
+      client_(client),
+      binding_(this),
+      weak_factory_(this) {
   DCHECK(web_contents);
   auto_signin_enabled_.Init(prefs::kCredentialsEnableAutosignin,
                             client_->GetPrefs());
 }
 
 CredentialManagerImpl::~CredentialManagerImpl() {}
 
 void CredentialManagerImpl::BindRequest(
     mojom::CredentialManagerRequest request) {
-  bindings_.AddBinding(this, std::move(request));
+  // This can happen if the pipe broke and the renderer is now reconnecting.
+  if (binding_.is_bound())
+    binding_.Close();

[dcheng, 2017/06/28 19:04:49]

I'm confused how we could get here--shouldn't the Disconnect() call below have
closed it before we get another binding request?

[engedy, 2017/06/28 19:26:34]

I was trying to be on the safe side here -- my thinking was that we could get
there if the Mojo pipe is broken for reasons other than either end closing it
purposely (some IPC error?).
Essentially, my thinking was that we could DCHECK(binding_.encountered_error())
here, but that flag is not exposed.

BindingStateBase::BindInternal DCHECK's that it's not bound, hence the if(),
although I'm not sure if anything exploded in production if we just called Bind
on a bound Binding anyway. WDYT?

[dcheng, 2017/06/29 06:43:49]

One common alternative is to add an explicit connection error handler that calls
Close() on the binding to reset the internal state on a connection error (if
either end of the pipe is closed for whatever reason). This is typically done if
there's other interesting work that should be reset when the message pipe is
closed (other than just closing the binding itself).

I don't have a strong preference about whether we use the connection error
handler or use the CL as-is, but long-term, this definitely seems like something
that makes more sense to be frame-scoped (once frame is 1:1 with document): does
this seem reasonable?

[engedy, 2017/06/30 19:41:55]

Yep, added error handler.

+  binding_.Bind(std::move(request));
+}
+
+bool CredentialManagerImpl::HasBinding() const {
+  return binding_.is_bound();
+}
+
+void CredentialManagerImpl::DisconnectBinding() {
+  binding_.Close();
 }
 
 void CredentialManagerImpl::Store(const CredentialInfo& credential,
                                   StoreCallback callback) {
   DCHECK_NE(CredentialType::CREDENTIAL_TYPE_EMPTY, credential.type);
 
   if (password_manager_util::IsLoggingActive(client_)) {
     CredentialManagerLogger(client_->GetLogManager())
         .LogStoreCredential(web_contents()->GetLastCommittedURL(),
                             credential.type);
@@ skipping 215 matching lines @@
 PasswordStore::FormDigest CredentialManagerImpl::GetSynthesizedFormForOrigin()
     const {
   PasswordStore::FormDigest digest = {
       autofill::PasswordForm::SCHEME_HTML, std::string(),
       web_contents()->GetLastCommittedURL().GetOrigin()};
   digest.signon_realm = digest.origin.spec();
   return digest;
 }
 
 }  // namespace password_manager