Add hooks to content to request permission to connect to the PPAPI broker.

content/renderer/pepper/pepper_broker_impl.cc

Index: content/renderer/pepper/pepper_broker_impl.cc
diff --git a/content/renderer/pepper/pepper_broker_impl.cc b/content/renderer/pepper/pepper_broker_impl.cc
index 67fdd84d80a10d79fb2ecd3524ebb1df273d7282..6e5c5b5e71e8ba4736a2e54fe39df4f2dff34651 100644
--- a/content/renderer/pepper/pepper_broker_impl.cc
+++ b/content/renderer/pepper/pepper_broker_impl.cc
@@ -116,18 +116,7 @@ PepperBrokerImpl::PepperBrokerImpl(webkit::ppapi::PluginModule* plugin_module,
 }
 
 PepperBrokerImpl::~PepperBrokerImpl() {
-  // Report failure to all clients that had pending operations.
-  for (ClientMap::iterator i = pending_connects_.begin();
-       i != pending_connects_.end(); ++i) {
-    base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr = i->second;
-    if (weak_ptr) {
-      weak_ptr->BrokerConnected(
-          ppapi::PlatformFileToInt(base::kInvalidPlatformFileValue),
-          PP_ERROR_ABORTED);
-    }
-  }
-  pending_connects_.clear();
-
+  ReportFailureToClients();
   plugin_module_->SetBroker(NULL);
   plugin_module_ = NULL;
 }
@@ -146,13 +135,7 @@ void PepperBrokerImpl::Connect(webkit::ppapi::PPB_Broker_Impl* client) {
   // longer using it.
   AddRef();
 
-  if (!dispatcher_.get()) {
-    pending_connects_[client] = client->AsWeakPtr();
-    return;
-  }
-  DCHECK(pending_connects_.empty());
-
-  ConnectPluginToBroker(client);
+  pending_connects_[client].client = client->AsWeakPtr();
 }
 
 void PepperBrokerImpl::Disconnect(webkit::ppapi::PPB_Broker_Impl* client) {
@@ -179,7 +162,7 @@ void PepperBrokerImpl::Disconnect(webkit::ppapi::PPB_Broker_Impl* client) {
       bool stopped = delegate_->StopWaitingForBrokerConnection(this);
 
       // Verify the assumption that there are no references other than the one
-      // client holds, which will be released below.
+      // |client| holds, which will be released below.
       DCHECK(!stopped || HasOneRef());
     }
   }
@@ -193,26 +176,81 @@ void PepperBrokerImpl::OnBrokerChannelConnected(
     const IPC::ChannelHandle& channel_handle) {
   scoped_ptr<PepperBrokerDispatcherWrapper> dispatcher(
       new PepperBrokerDispatcherWrapper);
-  if (dispatcher->Init(channel_handle)) {
-    dispatcher_.reset(dispatcher.release());
-
-    // Process all pending channel requests from the plugins.
-    for (ClientMap::iterator i = pending_connects_.begin();
-         i != pending_connects_.end(); ++i) {
-      base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr = i->second;
-      if (weak_ptr)
-        ConnectPluginToBroker(weak_ptr);
+  if (!dispatcher->Init(channel_handle)) {
+    ReportFailureToClients();
+    return;
+  }
+
+  dispatcher_.reset(dispatcher.release());
+
+  // Process all pending channel requests from the plugins.
+  for (ClientMap::iterator i = pending_connects_.begin();
+       i != pending_connects_.end();) {
+    base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr =
+        i->second.client;
+    LOG_IF(ERROR, !weak_ptr) << "Client has gone away";

[ddorwin, 2012/08/12 22:51:16]

Seems more like a warning.

[Bernhard Bauer, 2012/08/13 09:02:12]

Oops, that was for debugging. Removed.

+    LOG_IF(ERROR, !i->second.authorized) << "Client is waiting for permission";

[ddorwin, 2012/08/12 22:51:16]

Is this really an error that should always be logged?

+    if (!i->second.authorized) {

[ddorwin, 2012/08/12 22:51:16]

Is the renderer process responsible for enforcing the result? It seems the
browser should be - otherwise, a compromised renderer can allow the broker
regardless of the user's choice.
I think to fully secure this, the browser must create the socket pair (currently
in  PepperBrokerImpl::ConnectPluginToBroker) and pass it back (i.e. via
OnBrokerPermissionResult()).

[Bernhard Bauer, 2012/08/13 09:02:12]

Yeah, I talked about this with Justin. AFAIU, a compromised renderer could
always lie about its URL to bypass the content settings, so I'm not sure how
much we would gain from having the decision in the browser. In the light of
that, we decided to do the simpler thing and keep the decision in the renderer.
For other plug-in blocking mechanisms (content settings, outdated and infobarred
plug-ins) we also make the final decision in the renderer instead of the
browser.

+      ++i;
+      continue;
     }
-  } else {
-    // Report failure to all clients.
-    for (ClientMap::iterator i = pending_connects_.begin();
-         i != pending_connects_.end(); ++i) {
-      base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr = i->second;
-      if (weak_ptr) {
-        weak_ptr->BrokerConnected(
-            ppapi::PlatformFileToInt(base::kInvalidPlatformFileValue),
-            PP_ERROR_FAILED);
-      }
+
+    if (weak_ptr)
+      ConnectPluginToBroker(weak_ptr);
+
+    pending_connects_.erase(i++);
+  }
+}
+
+void PepperBrokerImpl::OnBrokerPermissionResult(
+    webkit::ppapi::PPB_Broker_Impl* client,
+    bool result) {
+  ClientMap::iterator entry = pending_connects_.find(client);
+  if (entry == pending_connects_.end())
+    return;
+
+  if (!entry->second.client) {
+    // Client has gone away.
+    pending_connects_.erase(entry);
+    return;
+  }
+
+  if (!result) {
+    // Report failure.
+    client->BrokerConnected(
+        ppapi::PlatformFileToInt(base::kInvalidPlatformFileValue),
+        PP_ERROR_NOACCESS);
+    pending_connects_.erase(entry);
+    return;
+  }
+
+  if (dispatcher_.get()) {
+    ConnectPluginToBroker(client);
+    pending_connects_.erase(entry);
+    return;
+  }
+
+  // Mark the request as authorized, continue waiting for the broker
+  // connection.
+  DCHECK(!entry->second.authorized);
+  entry->second.authorized = true;
+}
+
+PepperBrokerImpl::PendingConnection::PendingConnection() : authorized(false) {
+}
+
+PepperBrokerImpl::PendingConnection::~PendingConnection() {
+}
+
+void PepperBrokerImpl::ReportFailureToClients() {
+  for (ClientMap::iterator i = pending_connects_.begin();
+       i != pending_connects_.end(); ++i) {
+    base::WeakPtr<webkit::ppapi::PPB_Broker_Impl>& weak_ptr =
+        i->second.client;
+    if (weak_ptr) {
+      weak_ptr->BrokerConnected(
+          ppapi::PlatformFileToInt(base::kInvalidPlatformFileValue),
+          PP_ERROR_ABORTED);
     }
   }
   pending_connects_.clear();