GuestView: Fix JavaScript Task Queue

The task queue in guest_view.js waits until the previous operation has called its callback before proceeding to the subsequent operation. If BrowserPlugin is torn down while guest_view.js is waiting for an operation to complete, it will fail to make progress.

This CL addresses this issue by calling the appropriate callbacks on tear down of ExtensionsGuestViewContainer.

This CL also makes sure that the destruction callback is registered in guest_view.js as soon as attach(..) is called rather than after we hear the callback. This ensures that guest_view.js sets its state correctly in the event of a tear down of BrowserPlugin between the call to attach and hearing the callback.

BUG=470456

Review URL: https://codereview.chromium.org/1039643003

Cr-Commit-Position: refs/heads/master@{#322652}

2 files changed, 50 insertions, 45 deletions

diff --git a/extensions/renderer/guest_view/extensions_guest_view_container.cc b/extensions/renderer/guest_view/extensions_guest_view_container.cc
index fe9570c..e5cdc57 100644
--- a/extensions/renderer/guest_view/extensions_guest_view_container.cc
+++ b/extensions/renderer/guest_view/extensions_guest_view_container.cc
@@ -40,6 +40,25 @@ ExtensionsGuestViewContainer::Request::GetCallback() const {
   return v8::Local<v8::Function>::New(isolate_, callback_);
 }
 
+void ExtensionsGuestViewContainer::Request::ExecuteCallbackIfAvailable(
+    int argc, scoped_ptr<v8::Handle<v8::Value>[]> argv) {
+  if (!HasCallback())
+    return;
+
+  v8::HandleScope handle_scope(isolate());
+  v8::Handle<v8::Function> callback = GetCallback();
+  v8::Handle<v8::Context> context = callback->CreationContext();
+  if (context.IsEmpty())
+    return;
+
+  v8::Context::Scope context_scope(context);
+  blink::WebScopedMicrotaskSuppression suppression;
+
+  // Call the AttachGuest API's callback with the guest proxy as the first
+  // parameter.
+  callback->Call(context->Global(), argc, argv.get());
+}
+
 ExtensionsGuestViewContainer::AttachRequest::AttachRequest(
     GuestViewContainer* container,
     int guest_instance_id,
@@ -74,10 +93,6 @@ void ExtensionsGuestViewContainer::AttachRequest::HandleResponse(
   if (!GuestViewMsg_GuestAttached::Read(&message, &param))
     return;
 
-  // If we don't have a callback then there's nothing more to do.
-  if (!HasCallback())
-    return;
-
   content::RenderView* guest_proxy_render_view =
       content::RenderView::FromRoutingID(get<1>(param));
   // TODO(fsamuel): Should we be reporting an error to JavaScript or DCHECKing?
@@ -85,23 +100,14 @@ void ExtensionsGuestViewContainer::AttachRequest::HandleResponse(
     return;
 
   v8::HandleScope handle_scope(isolate());
-  v8::Handle<v8::Function> callback = GetCallback();
-  v8::Handle<v8::Context> context = callback->CreationContext();
-  if (context.IsEmpty())
-    return;
-
   blink::WebFrame* frame = guest_proxy_render_view->GetWebView()->mainFrame();
   v8::Local<v8::Value> window = frame->mainWorldScriptContext()->Global();
 
   const int argc = 1;
-  v8::Handle<v8::Value> argv[argc] = { window };
-
-  v8::Context::Scope context_scope(context);
-  blink::WebScopedMicrotaskSuppression suppression;
+  scoped_ptr<v8::Handle<v8::Value>[]> argv(new v8::Handle<v8::Value>[argc]);
+  argv[0] = window;
 
-  // Call the AttachGuest API's callback with the guest proxy as the first
-  // parameter.
-  callback->Call(context->Global(), argc, argv);
+  ExecuteCallbackIfAvailable(argc, argv.Pass());
 }
 
 ExtensionsGuestViewContainer::DetachRequest::DetachRequest(
@@ -123,21 +129,7 @@ void ExtensionsGuestViewContainer::DetachRequest::PerformRequest() {
 
 void ExtensionsGuestViewContainer::DetachRequest::HandleResponse(
     const IPC::Message& message) {
-  // If we don't have a callback then there's nothing more to do.
-  if (!HasCallback())
-    return;
-
-  v8::HandleScope handle_scope(isolate());
-  v8::Handle<v8::Function> callback = GetCallback();
-  v8::Handle<v8::Context> context = callback->CreationContext();
-  if (context.IsEmpty())
-    return;
-
-  v8::Context::Scope context_scope(context);
-  blink::WebScopedMicrotaskSuppression suppression;
-
-  // Call the DetachGuest's callback.
-  callback->Call(context->Global(), 0 /* argc */, nullptr);
+  ExecuteCallbackIfAvailable(0 /* argc */, nullptr);
 }
 
 ExtensionsGuestViewContainer::ExtensionsGuestViewContainer(
@@ -150,24 +142,34 @@ ExtensionsGuestViewContainer::ExtensionsGuestViewContainer(
 }
 
 ExtensionsGuestViewContainer::~ExtensionsGuestViewContainer() {
-  if (element_instance_id() != guestview::kInstanceIDNone) {
+  if (element_instance_id() != guestview::kInstanceIDNone)
     g_guest_view_container_map.Get().erase(element_instance_id());
-  }
 
-  // Call the destruction callback, if one is registered.
-  if (destruction_callback_.IsEmpty())
-    return;
-  v8::HandleScope handle_scope(destruction_isolate_);
-  v8::Handle<v8::Function> callback =
-      v8::Local<v8::Function>::New(destruction_isolate_, destruction_callback_);
-  v8::Handle<v8::Context> context = callback->CreationContext();
-  if (context.IsEmpty())
-    return;
+  if (pending_response_.get())
+    pending_response_->ExecuteCallbackIfAvailable(0 /* argc */, nullptr);
 
-  v8::Context::Scope context_scope(context);
-  blink::WebScopedMicrotaskSuppression suppression;
+  while (pending_requests_.size() > 0) {
+    linked_ptr<Request> pending_request = pending_requests_.front();
+    pending_requests_.pop_front();
+    // Call the JavaScript callbacks with no arguments which implies an error.
+    pending_request->ExecuteCallbackIfAvailable(0 /* argc */, nullptr);
+  }
 
-  callback->Call(context->Global(), 0 /* argc */, nullptr);
+  // Call the destruction callback, if one is registered.
+  if (!destruction_callback_.IsEmpty()) {
+    v8::HandleScope handle_scope(destruction_isolate_);
+    v8::Handle<v8::Function> callback =
+        v8::Local<v8::Function>::New(destruction_isolate_,
+                                     destruction_callback_);
+    v8::Handle<v8::Context> context = callback->CreationContext();
+    if (context.IsEmpty())
+      return;
+
+    v8::Context::Scope context_scope(context);
+    blink::WebScopedMicrotaskSuppression suppression;
+
+    callback->Call(context->Global(), 0 /* argc */, nullptr);
+  }
 }
 
 ExtensionsGuestViewContainer* ExtensionsGuestViewContainer::FromID(
diff --git a/extensions/renderer/guest_view/extensions_guest_view_container.h b/extensions/renderer/guest_view/extensions_guest_view_container.h
index 7071514..75e2169 100644
--- a/extensions/renderer/guest_view/extensions_guest_view_container.h
+++ b/extensions/renderer/guest_view/extensions_guest_view_container.h
@@ -32,6 +32,9 @@ class ExtensionsGuestViewContainer : public GuestViewContainer {
     virtual void PerformRequest() = 0;
     virtual void HandleResponse(const IPC::Message& message) = 0;
 
+    void ExecuteCallbackIfAvailable(int argc,
+                                    scoped_ptr<v8::Handle<v8::Value>[]> argv);
+
     GuestViewContainer* container() const { return container_; }
 
     bool HasCallback() const;