[v8 gc] Introduce a base class for all objects that can have pending activity

third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 12 matching lines @@
  * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "bindings/core/v8/V8GCController.h"
 
+#include "bindings/core/v8/ActiveScriptWrappable.h"
 #include "bindings/core/v8/NPV8Object.h"
 #include "bindings/core/v8/RetainedDOMInfo.h"
 #include "bindings/core/v8/V8AbstractEventListener.h"
 #include "bindings/core/v8/V8Binding.h"
 #include "bindings/core/v8/V8MutationObserver.h"
 #include "bindings/core/v8/V8Node.h"
 #include "bindings/core/v8/V8ScriptRunner.h"
 #include "bindings/core/v8/WrapperTypeInfo.h"
 #include "core/dom/Attr.h"
 #include "core/dom/Document.h"
@@ skipping 69 matching lines @@
         // MinorGC does not collect objects because it may be expensive to
         // update references during minorGC
         if (classId == WrapperTypeInfo::ObjectClassId) {
             v8::Persistent<v8::Object>::Cast(*value).MarkActive();
             return;
         }
 
         v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New(m_isolate, v8::Persistent<v8::Object>::Cast(*value));
         ASSERT(V8DOMWrapper::hasInternalFieldsSet(wrapper));
         const WrapperTypeInfo* type = toWrapperTypeInfo(wrapper);
-        if (type != npObjectTypeInfo() && toScriptWrappable(wrapper)->hasPendingActivity()) {
+        if (type != npObjectTypeInfo() && toActiveScriptWrappable(wrapper) && toActiveScriptWrappable(wrapper)->hasPendingActivity()) {

[Yuki, 2016/03/18 06:59:40]

I think you can do:
    type != npObjectTypeInfo() &&
    type->toActiveScriptWrappable(wrapper) &&
    type->toActiveScriptWrappable(wrapper)->hasPendingActivity()
and then, you don't need the new global function:
    toActiveScriptWrappable(wrapper)

In addition, I think we'd like to have hasPendingActivity() in WrapperTypeInfo.
    type != npObjectTypeInfo() &&
    type->hasPendingActivity(wrapper)
then, we no longer need to call toActiveScriptWrappable() twice.  It should be
faster.

[jochen (gone - plz use gerrit), 2016/03/18 13:18:01]

added hasPendingActivity()

             v8::Persistent<v8::Object>::Cast(*value).MarkActive();
             return;
         }
 
         if (classId == WrapperTypeInfo::NodeClassId) {
             ASSERT(V8Node::hasInstance(wrapper, m_isolate));
             Node* node = V8Node::toImpl(wrapper);
             if (node->hasEventListeners()) {
                 v8::Persistent<v8::Object>::Cast(*value).MarkActive();
                 return;
@@ skipping 24 matching lines @@
 
     void VisitPersistentHandle(v8::Persistent<v8::Value>* value, uint16_t classId) override
     {
         if (classId != WrapperTypeInfo::NodeClassId && classId != WrapperTypeInfo::ObjectClassId)
             return;
 
         v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New(m_isolate, v8::Persistent<v8::Object>::Cast(*value));
         ASSERT(V8DOMWrapper::hasInternalFieldsSet(wrapper));
 
         const WrapperTypeInfo* type = toWrapperTypeInfo(wrapper);
-        if (type != npObjectTypeInfo() && toScriptWrappable(wrapper)->hasPendingActivity()) {
+        if (type != npObjectTypeInfo() && toActiveScriptWrappable(wrapper) && toActiveScriptWrappable(wrapper)->hasPendingActivity()) {
             // If you hit this assert, you'll need to add a [DependentiLifetime]
             // extended attribute to the DOM interface. A DOM interface that
             // overrides hasPendingActivity must be marked as [DependentiLifetime].
             RELEASE_ASSERT(!value->IsIndependent());
             m_isolate->SetObjectGroupId(*value, liveRootId());
             ++m_domObjectsWithPendingActivity;
         }
 
         if (value->IsIndependent())
             return;
@@ skipping 251 matching lines @@
 };
 
 void V8GCController::traceDOMWrappers(v8::Isolate* isolate, Visitor* visitor)
 {
     DOMWrapperTracer tracer(visitor);
     isolate->VisitHandlesWithClassIds(&tracer);
 }
 
 class PendingActivityVisitor : public v8::PersistentHandleVisitor {
 public:
-    explicit PendingActivityVisitor(ExecutionContext* executionContext)
-        : m_executionContext(executionContext)
+    PendingActivityVisitor(v8::Isolate* isolate, ExecutionContext* executionContext)
+        : m_isolate(isolate)
+        , m_executionContext(executionContext)
         , m_pendingActivityFound(false)
     {
     }
 
     void VisitPersistentHandle(v8::Persistent<v8::Value>* value, uint16_t classId) override
     {
         // If we have already found any wrapper that has a pending activity,
         // we don't need to check other wrappers.
         if (m_pendingActivityFound)
             return;
 
         if (classId != WrapperTypeInfo::NodeClassId && classId != WrapperTypeInfo::ObjectClassId)
             return;
 
-        const v8::Persistent<v8::Object>& wrapper = v8::Persistent<v8::Object>::Cast(*value);
+        v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New(m_isolate, v8::Persistent<v8::Object>::Cast(*value));
+        ASSERT(V8DOMWrapper::hasInternalFieldsSet(wrapper));
         const WrapperTypeInfo* type = toWrapperTypeInfo(wrapper);
         // The ExecutionContext check is heavy, so it should be done at the last.
         if (type != npObjectTypeInfo()
-            && toScriptWrappable(wrapper)->hasPendingActivity()
+            && toActiveScriptWrappable(wrapper) && toActiveScriptWrappable(wrapper)->hasPendingActivity()
             // TODO(haraken): Currently we don't have a way to get a creation
             // context from a wrapper. We should implement the way and enable
             // the following condition.
             //
             // This condition affects only compositor workers, where one isolate
             // is shared by multiple workers. If we don't have the condition,
             // a worker object for a compositor worker doesn't get collected
             // until all compositor workers in the same isolate lose pending
             // activities. In other words, not having the condition delays
             // destruction of a worker object of a compositor worker.
             //
             /* && toExecutionContext(wrapper->creationContext()) == m_executionContext */
             )
             m_pendingActivityFound = true;
     }
 
     bool pendingActivityFound() const { return m_pendingActivityFound; }
 
 private:
+    v8::Isolate* m_isolate;
     RawPtrWillBePersistent<ExecutionContext> m_executionContext;
     bool m_pendingActivityFound;
 };
 
-bool V8GCController::hasPendingActivity(ExecutionContext* executionContext)
+bool V8GCController::hasPendingActivity(v8::Isolate* isolate, ExecutionContext* executionContext)

[Marcel Hlopko, 2016/03/18 08:34:40]

Pardon my lack of knowledge of the system, but I'm wondering whether we need
isolate as a parameter here, can't we just take the "current" one? Can one
isolate ask if other isolate hasPendingActivity? Feel free to educate me offline
if it's not worth typing :)

[jochen (gone - plz use gerrit), 2016/03/18 13:18:02]

the embedder should always know which isolate it wants to access

 {
     // V8GCController::hasPendingActivity is used only when a worker checks if
     // the worker contains any wrapper that has pending activities.
     ASSERT(!isMainThread());
 
     DEFINE_THREAD_SAFE_STATIC_LOCAL(CustomCountHistogram, scanPendingActivityHistogram, new CustomCountHistogram("Blink.ScanPendingActivityDuration", 1, 1000, 50));
     double startTime = WTF::currentTimeMS();
-    PendingActivityVisitor visitor(executionContext);
+    PendingActivityVisitor visitor(isolate, executionContext);
     toIsolate(executionContext)->VisitHandlesWithClassIds(&visitor);
     scanPendingActivityHistogram.count(static_cast<int>(WTF::currentTimeMS() - startTime));
     return visitor.pendingActivityFound();
 }
 
 } // namespace blink