Bypass the v8 GC second pass callback set up by GCCallback for extension messaging.

This fixes a bug where an extension script context was being invalidated in
between calls for the first and second pass v8::PersistentBase::SetWeak
callbacks, which would invalidate a base::WeakPtr and crash. This bug can be
trivially bypassed by removing the second pass entirely and relying on the post
back to the current message loop.

This also adds 2 tests, one where the script context is invalidated before GC,
the other where GC happens before the script context is invalidated.

This patch is a combination of jochen's https://crrev.com/1247093005 and
kalman's https://crrev.com/1242273007, see those patches for more comments.

BUG=512080
R=rdevlin.cronin@chromium.org,jochen@chromium.org,adamk@chromium.org

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

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

1 files changed, 1 insertions, 66 deletions

diff --git a/extensions/renderer/messaging_bindings.cc b/extensions/renderer/messaging_bindings.cc
index 4017309..1e0f6e2 100644
--- a/extensions/renderer/messaging_bindings.cc
+++ b/extensions/renderer/messaging_bindings.cc
@@ -25,6 +25,7 @@
 #include "extensions/common/manifest_handlers/externally_connectable.h"
 #include "extensions/renderer/dispatcher.h"
 #include "extensions/renderer/event_bindings.h"
+#include "extensions/renderer/gc_callback.h"
 #include "extensions/renderer/object_backed_native_handler.h"
 #include "extensions/renderer/script_context.h"
 #include "extensions/renderer/script_context_set.h"
@@ -58,72 +59,6 @@ using v8_helpers::IsEmptyOrUndefied;
 
 namespace {
 
-// Binds |callback| to run when |object| is garbage collected. So as to not
-// re-entrantly call into v8, we execute this function asynchronously, at
-// which point |context| may have been invalidated. If so, |callback| is not
-// run, and |fallback| will be called instead.
-//
-// Deletes itself when the object args[0] is garbage collected or when the
-// context is invalidated.
-class GCCallback : public base::SupportsWeakPtr<GCCallback> {
- public:
-  GCCallback(ScriptContext* context,
-             const v8::Local<v8::Object>& object,
-             const v8::Local<v8::Function>& callback,
-             const base::Closure& fallback)
-      : context_(context),
-        object_(context->isolate(), object),
-        callback_(context->isolate(), callback),
-        fallback_(fallback) {
-    object_.SetWeak(this, FirstWeakCallback, v8::WeakCallbackType::kParameter);
-    context->AddInvalidationObserver(
-        base::Bind(&GCCallback::OnContextInvalidated, AsWeakPtr()));
-  }
-
- private:
-  static void FirstWeakCallback(const v8::WeakCallbackInfo<GCCallback>& data) {
-    // v8 says we need to explicitly reset weak handles from their callbacks.
-    // It's not implicit as one might expect.
-    data.GetParameter()->object_.Reset();
-    data.SetSecondPassCallback(SecondWeakCallback);
-  }
-
-  static void SecondWeakCallback(const v8::WeakCallbackInfo<GCCallback>& data) {
-    base::MessageLoop::current()->PostTask(
-        FROM_HERE,
-        base::Bind(&GCCallback::RunCallback, data.GetParameter()->AsWeakPtr()));
-  }
-
-  void RunCallback() {
-    CHECK(context_);
-    v8::Isolate* isolate = context_->isolate();
-    v8::HandleScope handle_scope(isolate);
-    context_->CallFunction(v8::Local<v8::Function>::New(isolate, callback_));
-    delete this;
-  }
-
-  void OnContextInvalidated() {
-    fallback_.Run();
-    context_ = NULL;
-    delete this;
-  }
-
-  // ScriptContext which owns this GCCallback.
-  ScriptContext* context_;
-
-  // Holds a global handle to the object this GCCallback is bound to.
-  v8::Global<v8::Object> object_;
-
-  // Function to run when |object_| bound to this GCCallback is GC'd.
-  v8::Global<v8::Function> callback_;
-
-  // Function to run if context is invalidated before we have a chance
-  // to execute |callback_|.
-  base::Closure fallback_;
-
-  DISALLOW_COPY_AND_ASSIGN(GCCallback);
-};
-
 // Tracks every reference between ScriptContexts and Ports, by ID.
 class PortTracker {
  public: