<webview>: First stab at implementing media permission request for guests.

content/renderer/browser_plugin/browser_plugin.cc

Index: content/renderer/browser_plugin/browser_plugin.cc
diff --git a/content/renderer/browser_plugin/browser_plugin.cc b/content/renderer/browser_plugin/browser_plugin.cc
index 47de906da2c4f3b73d9ab92bed608d4aa995d4c8..b2bfa822f6724e73beeadc71de83c30172761576 100644
--- a/content/renderer/browser_plugin/browser_plugin.cc
+++ b/content/renderer/browser_plugin/browser_plugin.cc
@@ -98,7 +98,9 @@ BrowserPlugin::BrowserPlugin(
       browser_plugin_manager_(render_view->browser_plugin_manager()),
       current_nav_entry_index_(0),
       nav_entry_count_(0),
-      compositing_enabled_(false) {
+      compositing_enabled_(false),
+      ALLOW_THIS_IN_INITIALIZER_LIST(
+          weak_ptr_factory_(this)) {
   bindings_.reset(new BrowserPluginBindings(this));
 }
 
@@ -129,6 +131,7 @@ bool BrowserPlugin::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(BrowserPluginMsg_LoadRedirect, OnLoadRedirect)
     IPC_MESSAGE_HANDLER(BrowserPluginMsg_LoadStart, OnLoadStart)
     IPC_MESSAGE_HANDLER(BrowserPluginMsg_LoadStop, OnLoadStop)
+    IPC_MESSAGE_HANDLER(BrowserPluginMsg_RequestPermission, OnRequestPermission)
     IPC_MESSAGE_HANDLER(BrowserPluginMsg_SetCursor, OnSetCursor)
     IPC_MESSAGE_HANDLER(BrowserPluginMsg_ShouldAcceptTouchEvents,
                         OnShouldAcceptTouchEvents)
@@ -387,6 +390,22 @@ void BrowserPlugin::SetInstanceID(int instance_id) {
   navigate_src_sent_ = true;
 }
 
+void BrowserPlugin::PopulateAncestorList() {
+  if (!container())
+    return;
+
+  WebKit::WebNode ancestor = container()->element();
+  // Escape the <webview> shim if this BrowserPlugin has one.
+  WebKit::WebElement shim = ancestor.shadowHost();
+  if (!shim.isNull())
+    ancestor = shim;
+  ancestors_.clear();
+  while (!ancestor.isNull()) {
+    ancestors_.push_back(ancestor);
+    ancestor = ancestor.parentNode();
+  }
+}
+
 void BrowserPlugin::OnAdvanceFocus(int instance_id, bool reverse) {
   DCHECK(render_view_);
   render_view_->GetWebView()->advanceFocus(reverse);
@@ -512,6 +531,15 @@ void BrowserPlugin::OnLoadStop(int instance_id) {
   TriggerEvent(browser_plugin::kEventLoadStop, NULL);
 }
 
+void BrowserPlugin::OnRequestPermission(
+    int instance_id,
+    BrowserPluginPermissionType permission_type,
+    int request_id,
+    const base::DictionaryValue& request_info) {
+  if (permission_type == BrowserPluginPermissionTypeMedia)
+    RequestMediaPermission(request_id, request_info);
+}
+
 void BrowserPlugin::OnSetCursor(int instance_id, const WebCursor& cursor) {
   cursor_ = cursor;
 }
@@ -528,6 +556,48 @@ void BrowserPlugin::OnUpdatedName(int instance_id, const std::string& name) {
   UpdateDOMAttribute(browser_plugin::kAttributeName, name);
 }
 
+void BrowserPlugin::RequestMediaPermission(
+    int request_id, const base::DictionaryValue& request_info) {
+  if (!HasEventListeners(browser_plugin::kEventRequestPermission)) {
+    // Automatically deny the request if there are no event listeners for
+    // permissionrequest.
+    AllowPermission(

[abarth-chromium, 2013/02/20 20:46:44]

It's confusing that you're using a function called "allow permission" to deny
the permission.

[lazyboy, 2013/02/20 22:09:36]

changed to "DecidePermission".

+        BrowserPluginPermissionTypeMedia, request_id, false /* allow */);

[abarth-chromium, 2013/02/20 20:46:44]

It's confusing that you're using a function called "AllowPermission" to deny the
permission.

[lazyboy, 2013/02/20 22:09:36]

ditto.

+    return;
+  }
+  DCHECK(!pending_permission_requests_.count(request_id));
+  pending_permission_requests_.insert(
+      std::make_pair(request_id,
+                     std::make_pair(request_id,
+                                    BrowserPluginPermissionTypeMedia)));
+
+  std::map<std::string, base::Value*> props;
+  props[browser_plugin::kPermission] =
+      base::Value::CreateStringValue(browser_plugin::kPermissionTypeMedia);
+  props[browser_plugin::kRequestId] =
+      base::Value::CreateIntegerValue(request_id);

[abarth-chromium, 2013/02/20 20:46:44]

Does this leak memory?  If not, where is this memory freed?

[lazyboy, 2013/02/20 22:09:36]

Seems we have this leak in all places where we're calling TriggerEvent, let me
fix that in a separate smaller patch.

[lazyboy, 2013/02/20 23:15:33]

I was wrong on this one, TriggerEvent() already frees up the memory by creating
DictionaryValue from |props|.

+
+  // Fill in the info provided by the browser.
+  for (DictionaryValue::Iterator iter(request_info); !iter.IsAtEnd();
+           iter.Advance()) {
+    props[iter.key()] = iter.value().DeepCopy();
+  }
+  TriggerEvent(browser_plugin::kEventRequestPermission, &props);
+}
+
+bool BrowserPlugin::HasEventListeners(const std::string& event_name) {
+  if (!container())
+    return false;
+
+  for (size_t i = 0; i < ancestors_.size(); ++i) {
+    if (ancestors_[i].hasEventListeners(
+            WebKit::WebString::fromUTF8(event_name))) {

[abarth-chromium, 2013/02/20 20:46:44]

It seems like we should create the WebString once instead of for each step in
the ancestor chain.

I think you're going to have to get rid of ancestors_ and walk the ancestor
chain directly here.

I'm not this it makes sense to check whether the ancestors have event listeners.
 That seems like a premature performance optimization.

[lazyboy, 2013/02/20 22:09:36]

Fixed WebString creation inside loop.
Removed ancestors_.

"I'm not this it makes sense to check whether the ancestors have event
listeners." ->
If we want to make this event bubble-able so any ancestor can also register for
this event; would we not have to do that?

+      return true;
+    }
+  }
+  return false;
+}
+
 void BrowserPlugin::OnUpdateRect(
     int instance_id,
     const BrowserPluginMsg_UpdateRect_Params& params) {
@@ -775,6 +845,73 @@ void BrowserPlugin::TriggerEvent(const std::string& event_name,
   container()->element().dispatchEvent(event);
 }
 
+void BrowserPlugin::OnRequestObjectGarbageCollected(int request_id) {
+  // Remove from alive objects.
+  std::map<int, AliveV8PermissionRequestItem*>::iterator iter =
+      alive_v8_permission_request_objects_.find(request_id);
+  if (iter != alive_v8_permission_request_objects_.end()) {
+    DCHECK(iter->second->first == request_id);
+    alive_v8_permission_request_objects_.erase(iter);
+  }
+
+  // If a decision has not been made for this request yet, deny it.
+  AllowPermissionIfRequestIsPending(request_id, false /*allow*/);
+}
+
+void BrowserPlugin::PersistRequestObject(
+    const NPVariant* request, const std::string& type, int id) {
+  DCHECK(alive_v8_permission_request_objects_.find(id) ==
+      alive_v8_permission_request_objects_.end());
+  if (alive_v8_permission_request_objects_.find(id) !=
+      alive_v8_permission_request_objects_.end()) {
+    return;
+  }

[abarth-chromium, 2013/02/20 20:46:44]

Why are you DCHECKing a condition and then handling it's negation?  Perhaps we
should use CHECK rather than DCHECK?

[lazyboy, 2013/02/20 22:09:36]

Changed to CHECK.

+  if (pending_permission_requests_.find(id) ==
+      pending_permission_requests_.end()) {
+    return;
+  }
+
+  v8::Persistent<v8::Value> weak_request =
+      v8::Persistent<v8::Value>::New(WebKit::WebBindings::toV8Value(request));
+
+  AliveV8PermissionRequestItem* new_item =
+      new std::pair<int, base::WeakPtr<BrowserPlugin> >(
+          id, weak_ptr_factory_.GetWeakPtr());
+
+  std::pair<std::map<int, AliveV8PermissionRequestItem*>::iterator, bool>
+      result = alive_v8_permission_request_objects_.insert(
+          std::make_pair(id, new_item));
+  CHECK(result.second);  // Inserted in the map.
+  AliveV8PermissionRequestItem* request_item = result.first->second;
+  weak_request.MakeWeak(request_item, WeakCallbackForPersistObject);
+}
+
+// static
+void BrowserPlugin::WeakCallbackForPersistObject(
+    v8::Persistent<v8::Value> object, void* param) {
+  v8::Persistent<v8::Object> persistent_object =
+      v8::Persistent<v8::Object>::Cast(object);
+
+  AliveV8PermissionRequestItem* item_ptr =
+      static_cast<AliveV8PermissionRequestItem*>(param);
+  int request_id = item_ptr->first;
+  base::WeakPtr<BrowserPlugin> plugin = item_ptr->second;
+  delete item_ptr;

[abarth-chromium, 2013/02/20 20:46:44]

I'm sad we need to manually manage memory like this.  Does Chromium really not
have better tools for memory management?

[lazyboy, 2013/02/20 22:09:36]

I'm not sure, the closest I could find was STLValueDeleter/STLElementDeleter in
stl_util.h, which doesn't fit our requirement here.

+
+  persistent_object.Dispose();
+  persistent_object.Clear();
+
+  if (plugin) {
+    // Asynchronously remove item from |alive_v8_permission_request_objects_|.
+    // Note that we are using weak pointer for the following PostTask, so we
+    // don't need to worry about BrowserPlugin going away.
+    MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&BrowserPlugin::OnRequestObjectGarbageCollected,
+                   plugin, request_id));
+  }
+}
+
 void BrowserPlugin::Back() {
   if (!navigate_src_sent_)
     return;
@@ -845,9 +982,44 @@ WebKit::WebPluginContainer* BrowserPlugin::container() const {
   return container_;
 }
 
+void BrowserPlugin::AllowPermission(
+    BrowserPluginPermissionType permission_type, int request_id, bool allow) {
+  switch (permission_type) {
+    case BrowserPluginPermissionTypeMedia:
+      browser_plugin_manager()->Send(
+          new BrowserPluginHostMsg_AllowPermission(
+              render_view_->GetRoutingID(), instance_id_,
+              BrowserPluginPermissionTypeMedia, request_id, allow));
+      break;
+    case BrowserPluginPermissionTypeUnknown:
+    default:
+      LOG(INFO) << "Not a valid permission type";
+      break;

[abarth-chromium, 2013/02/20 20:46:44]

Should we have a DCHECK or a CHECK here?  How bad an error is this?

[lazyboy, 2013/02/20 22:09:36]

Added DCHECK.
This function is "supposed to" be accessible from web_view.js only, so an
incorrect permission_type is wrong, but should not be harmful; DCHECK is better
in these case, right?

+  }
+}
+
+void BrowserPlugin::AllowPermissionIfRequestIsPending(
+    int request_id, bool allow) {
+  PendingPermissionRequests::iterator iter =
+      pending_permission_requests_.find(request_id);
+  if (iter == pending_permission_requests_.end())
+    return;
+
+  int type = iter->second.second;
+  pending_permission_requests_.erase(iter);
+  AllowPermission(type, request_id, allow);
+}
+
+void BrowserPlugin::OnEmbedderDecidedPermission(int request_id, bool allow) {
+  AllowPermissionIfRequestIsPending(request_id, allow);
+}
+
 bool BrowserPlugin::initialize(WebPluginContainer* container) {
   container_ = container;
   container_->setWantsWheelEvents(true);
+  // This ancestor set is valid for the lifetime of BrowserPlugin as this object
+  // does not survive reparenting in the DOM tree.
+  PopulateAncestorList();
   ParseAttributes();
   return true;
 }