Oilpan: make SuspendableScriptExecutor safe.

Source/web/SuspendableScriptExecutor.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "web/SuspendableScriptExecutor.h"
 
 #include "bindings/core/v8/ScriptController.h"
 #include "bindings/core/v8/ScriptSourceCode.h"
 #include "core/dom/Document.h"
@@ skipping 10 matching lines @@
     executor->ref();
     executor->run();
 }
 
 void SuspendableScriptExecutor::contextDestroyed()
 {
     // this method can only be called if the script was not called in run()
     // and context remained suspend (method resume has never called)
     SuspendableTimer::contextDestroyed();
     m_callback->completed(Vector<v8::Local<v8::Value>>());
-    deref();
+    dispose();
 }
 
 SuspendableScriptExecutor::SuspendableScriptExecutor(LocalFrame* frame, int worldID, const WillBeHeapVector<ScriptSourceCode>& sources, int extensionGroup, bool userGesture, WebScriptExecutionCallback* callback)
     : SuspendableTimer(frame->document())
     , m_frame(frame)
     , m_worldID(worldID)
     , m_sources(sources)
     , m_extensionGroup(extensionGroup)
     , m_userGesture(userGesture)
     , m_callback(callback)
@@ skipping 32 matching lines @@
 
     v8::HandleScope scope(v8::Isolate::GetCurrent());
     Vector<v8::Local<v8::Value>> results;
     if (m_worldID) {
         m_frame->script().executeScriptInIsolatedWorld(m_worldID, m_sources, m_extensionGroup, &results);
     } else {
         v8::Local<v8::Value> scriptValue = m_frame->script().executeScriptInMainWorldAndReturnValue(m_sources.first());
         results.append(scriptValue);
     }
     m_callback->completed(results);
+    dispose();
+}
+
+void SuspendableScriptExecutor::dispose()
+{
+#if ENABLE(OILPAN)
+    // Remove object as a ContextLifecycleObserver.
+    ActiveDOMObject::clearContext();

[haraken, 2015/06/23 09:25:56]

Is this a performance optimization? Or do we need to explicitly remove the
observer for correctness?

(It seems a bit strange to call clearContext in contextDestroyed.)

[sof, 2015/06/23 09:40:59]

I think it makes sense to remove oneself here, rather than keep a weak reference
lingering. We know this observer's lifetime is up.

[haraken, 2015/06/23 09:44:55]

Makes sense. (I'd prefer calling ActiveDOMObject::clearContext() only in
executeAndDestroySelf() though; it's up to you.)

+#endif
     deref();
 }
 
 DEFINE_TRACE(SuspendableScriptExecutor)
 {
 #if ENABLE(OILPAN)
     visitor->trace(m_frame);
     visitor->trace(m_sources);
 #endif
     SuspendableTimer::trace(visitor);
 }
 
 } // namespace blink