Restrict CM API interface request and message dispatch.

components/password_manager/content/browser/credential_manager_impl.cc

Index: components/password_manager/content/browser/credential_manager_impl.cc
diff --git a/components/password_manager/content/browser/credential_manager_impl.cc b/components/password_manager/content/browser/credential_manager_impl.cc
index d78025d7226635b14e1f21be113fe5a3d3360591..47c77770dcdf6c27846ed7882cbdd00dc8b1d9c3 100644
--- a/components/password_manager/content/browser/credential_manager_impl.cc
+++ b/components/password_manager/content/browser/credential_manager_impl.cc
@@ -40,7 +40,10 @@ void RunMojoGetCallback(mojom::CredentialManager::GetCallback callback,
 
 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());
@@ -50,7 +53,18 @@ 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,