Revert of Add tests for trace wrappers (patchset #15 id:280001 of...

Revert of Add tests for trace wrappers (patchset #15 id:280001 of https://codereview.chromium.org/2301213003/ )

Reason for revert:
Sorry for the revert; this patch has (I believe) caused the heap-snapshot-with-detached-dom-tree.html to start failing.

The very first failure in that test was in https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty/builds/17518 -- immediately after this CL was committed.

Here is the flakiness dashboard result for that test:

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=inspector-protocol%2Fheap-profiler%2Fheap-snapshot-with-detached-dom-tree.html&testType=webkit_tests

The test is failing occasionally with output like this:

    Test that all nodes from the detached DOM tree will get into one group in the heap snapshot. Bug 107819.

     Took heap snapshot
    Parsed snapshot
    SUCCESS: found (Detached DOM trees)
    SUCCESS: found Detached DOM tree / 3 entries
    FAIL: unexpected DIV count: 1

Reverting this CL since it appears to be primarily about the tests.

Original issue's description:
> Add tests for trace wrappers
>
> This cl adds very basic tests for general trace wrappers features. And enables TraceWrappables runtime enabled feature for tests!
>
> LOG=no
> BUG=468240
>
> Committed: https://crrev.com/f7d5d042289c5fbe918348830917b888ec587272
> Cr-Commit-Position: refs/heads/master@{#419298}

TBR=haraken@chromium.org,mlippautz@chromium.org,hlopko@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=468240

Review-Url: https://codereview.chromium.org/2350733002
Cr-Commit-Position: refs/heads/master@{#419463}

third_party/WebKit/LayoutTests/fast/dom/NodeList/nodelist-reachable-expected.txt

@@ -2,7 +2,7 @@ PASS document.body.childNodes.customProperty is 1
 PASS document.getElementsByClassName("class").customProperty is 2
 PASS document.getElementsByName("name").customProperty is 3
 PASS document.getElementsByTagName("body").customProperty is 4
-PASS document.querySelector("form").elements["radios"].customProperty is 5
+FAIL document.querySelector("form").elements["radios"].customProperty should be 5 (of type number). Was undefined (of type undefined).
 PASS document.querySelector("input").labels.customProperty is 6
 PASS successfullyParsed is true
 

third_party/WebKit/LayoutTests/inspector-protocol/heap-profiler/heap-snapshot-with-active-dom-object-expected.txt

@@ -3,5 +3,7 @@ Test that all ActiveDOMObjects with pending activities will get into one group i
 
  Took heap snapshot
 Parsed snapshot
-FAIL: cannot find '(Pending activities group)'
+SUCCESS: found (Pending activities group)
+SUCCESS: found Pending activities / 3 entries
+SUCCESS: found 2 MediaQueryLists in Pending activities / 3 entries
 

third_party/WebKit/LayoutTests/inspector-protocol/heap-profiler/heap-snapshot-with-detached-dom-tree-expected.txt

@@ -3,5 +3,6 @@ Test that all nodes from the detached DOM tree will get into one group in the he
  Took heap snapshot
 Parsed snapshot
 SUCCESS: found (Detached DOM trees)
-FAIL: unexpected detached DOM tree: Detached DOM tree / 2 entries
+SUCCESS: found Detached DOM tree / 3 entries
+SUCCESS: found 3 DIVs in Detached DOM tree / 3 entries
 

third_party/WebKit/LayoutTests/inspector-protocol/heap-profiler/heap-snapshot-with-event-listener-expected.txt

@@ -3,5 +3,5 @@ Test that all nodes from the detached DOM tree will get into one group in the he
  Took heap snapshot
 Parsed snapshot
 SUCCESS: found myEventListener
-FAIL: cannot find HTMLBodyElement among retainers of (Global handles)
+SUCCESS: found link from HTMLBodyElement to HTMLBodyElement
 

third_party/WebKit/Source/bindings/bindings.gni

@@ -230,7 +230,6 @@ bindings_unittest_files =
                     "core/v8/ScriptPromiseTest.cpp",
                     "core/v8/ScriptStreamerTest.cpp",
                     "core/v8/ScriptValueSerializerTest.cpp",
-                    "core/v8/ScriptWrappableVisitorTest.cpp",
                     "core/v8/SerializedScriptValueTest.cpp",
                     "core/v8/ToV8Test.cpp",
                     "core/v8/V8BindingForTesting.cpp",

third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.h

@@ -30,17 +30,17 @@ public:
         const void* object)
         : m_traceWrappersCallback(traceWrappersCallback)
         , m_heapObjectHeaderCallback(heapObjectHeaderCallback)
-        , m_rawObjectPointer(object)
+        , m_object(object)
     {
         DCHECK(m_traceWrappersCallback);
         DCHECK(m_heapObjectHeaderCallback);
-        DCHECK(m_rawObjectPointer);
+        DCHECK(m_object);
     }
 
     inline void traceWrappers(WrapperVisitor* visitor)
     {
-        if (m_rawObjectPointer) {
-            m_traceWrappersCallback(visitor, m_rawObjectPointer);
+        if (m_object) {
+            m_traceWrappersCallback(visitor, m_object);
         }
     }
 
@@ -50,35 +50,29 @@ public:
      */
     inline bool isWrapperHeaderMarked()
     {
-        return !m_rawObjectPointer || heapObjectHeader()->isWrapperHeaderMarked();
+        return !m_object || heapObjectHeader()->isWrapperHeaderMarked();
     }
 
-    /**
-     * Returns raw object pointer. Beware it doesn't necessarily point to the
-     * beginning of the object.
-     */
-    const void* rawObjectPointer() { return m_rawObjectPointer; }
-
 private:
     inline bool shouldBeInvalidated()
     {
-        return m_rawObjectPointer && !heapObjectHeader()->isMarked();
+        return m_object && !heapObjectHeader()->isMarked();
     }
 
     inline void invalidate()
     {
-        m_rawObjectPointer = nullptr;
+        m_object = nullptr;
     }
 
     inline const HeapObjectHeader* heapObjectHeader()
     {
-        DCHECK(m_rawObjectPointer);
-        return m_heapObjectHeaderCallback(m_rawObjectPointer);
+        DCHECK(m_object);
+        return m_heapObjectHeaderCallback(m_object);
     }
 
     void (*m_traceWrappersCallback)(const WrapperVisitor*, const void*);
     HeapObjectHeader* (*m_heapObjectHeaderCallback)(const void*);
-    const void* m_rawObjectPointer;
+    const void* m_object;
 };
 
 /**
@@ -157,10 +151,6 @@ public:
     void markWrappersInAllWorlds(const ScriptWrappable*) const override;
     void markWrappersInAllWorlds(const void*) const override {}
 
-    WTF::Deque<WrapperMarkingData>* getMarkingDeque() { return &m_markingDeque; }
-    WTF::Deque<WrapperMarkingData>* getVerifierDeque() { return &m_verifierDeque; }
-    WTF::Vector<HeapObjectHeader*>* getHeadersToUnmark() { return &m_headersToUnmark; }
-
 private:
     /**
      * Is wrapper tracing currently in progress? True if TracePrologue has been

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

-// Copyright 2016 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 "bindings/core/v8/ScriptWrappableVisitor.h"
-
-#include "bindings/core/v8/ToV8.h"
-#include "bindings/core/v8/V8BindingForTesting.h"
-#include "bindings/core/v8/V8GCController.h"
-#include "bindings/core/v8/V8PerIsolateData.h"
-#include "core/testing/DeathAwareScriptWrappable.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace blink {
-
-static void preciselyCollectGarbage()
-{
-    ThreadState::current()->collectAllGarbage();
-}
-
-static void runV8Scavenger(v8::Isolate* isolate)
-{
-    V8GCController::collectGarbage(isolate, true);
-}
-
-static void runV8FullGc(v8::Isolate* isolate)
-{
-    V8GCController::collectGarbage(isolate, false);
-}
-
-TEST(ScriptWrappableVisitorTest, OilpanCollectObjectsNotReachableFromV8)
-{
-    V8TestingScope scope;
-    v8::Isolate* isolate = scope.isolate();
-
-    {
-        v8::HandleScope handleScope(isolate);
-        DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::create();
-        DeathAwareScriptWrappable::observeDeathsOf(object);
-
-        // Creates new V8 wrapper and associates it with global scope
-        toV8(object, scope.context()->Global(), isolate);
-    }
-
-    runV8Scavenger(isolate);
-    runV8FullGc(isolate);
-    preciselyCollectGarbage();
-
-    EXPECT_TRUE(DeathAwareScriptWrappable::hasDied());
-}
-
-TEST(ScriptWrappableVisitorTest, OilpanDoesntCollectObjectsReachableFromV8)
-{
-    V8TestingScope scope;
-    v8::Isolate* isolate = scope.isolate();
-    v8::HandleScope handleScope(isolate);
-    DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::create();
-    DeathAwareScriptWrappable::observeDeathsOf(object);
-
-    // Creates new V8 wrapper and associates it with global scope
-    toV8(object, scope.context()->Global(), isolate);
-
-    runV8Scavenger(isolate);
-    runV8FullGc(isolate);
-    preciselyCollectGarbage();
-
-    EXPECT_FALSE(DeathAwareScriptWrappable::hasDied());
-}
-
-TEST(ScriptWrappableVisitorTest, V8ReportsLiveObjectsDuringScavenger)
-{
-    V8TestingScope scope;
-    v8::Isolate* isolate = scope.isolate();
-    v8::HandleScope handleScope(isolate);
-    DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::create();
-    DeathAwareScriptWrappable::observeDeathsOf(object);
-
-    v8::Local<v8::Value> wrapper = toV8(
-        object, scope.context()->Global(), isolate);
-    EXPECT_TRUE(wrapper->IsObject());
-    v8::Local<v8::Object> wrapperObject = wrapper->ToObject();
-    // V8 collects wrappers with unmodified maps (as they can be recreated
-    // without loosing any data if needed). We need to create some property on
-    // wrapper so V8 will not see it as unmodified.
-    EXPECT_TRUE(wrapperObject->CreateDataProperty(scope.context(), 1, wrapper).IsJust());
-
-    runV8Scavenger(isolate);
-    preciselyCollectGarbage();
-
-    EXPECT_FALSE(DeathAwareScriptWrappable::hasDied());
-}
-
-TEST(ScriptWrappableVisitorTest, V8ReportsLiveObjectsDuringFullGc)
-{
-    V8TestingScope scope;
-    v8::Isolate* isolate = scope.isolate();
-    v8::HandleScope handleScope(isolate);
-    DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::create();
-    DeathAwareScriptWrappable::observeDeathsOf(object);
-
-    toV8(object, scope.context()->Global(), isolate);
-
-    runV8Scavenger(isolate);
-    runV8FullGc(isolate);
-    preciselyCollectGarbage();
-
-    EXPECT_FALSE(DeathAwareScriptWrappable::hasDied());
-}
-
-TEST(ScriptWrappableVisitorTest, OilpanClearsHeadersWhenObjectDied)
-{
-    V8TestingScope scope;
-
-    DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::create();
-    ScriptWrappableVisitor* visitor =
-        V8PerIsolateData::from(scope.isolate())->scriptWrappableVisitor();
-    auto header = HeapObjectHeader::fromPayload(object);
-    visitor->getHeadersToUnmark()->append(header);
-
-    preciselyCollectGarbage();
-
-    EXPECT_FALSE(visitor->getHeadersToUnmark()->contains(header));
-    visitor->getHeadersToUnmark()->clear();
-}
-
-TEST(ScriptWrappableVisitorTest, OilpanClearsMarkingDequeWhenObjectDied)
-{
-    V8TestingScope scope;
-
-    DeathAwareScriptWrappable* object = DeathAwareScriptWrappable::create();
-    ScriptWrappableVisitor* visitor =
-        V8PerIsolateData::from(scope.isolate())->scriptWrappableVisitor();
-    visitor->pushToMarkingDeque(
-        TraceTrait<DeathAwareScriptWrappable>::markWrapper,
-        TraceTrait<DeathAwareScriptWrappable>::heapObjectHeader,
-        object);
-
-    EXPECT_EQ(visitor->getMarkingDeque()->first().rawObjectPointer(), object);
-
-    preciselyCollectGarbage();
-
-    EXPECT_EQ(visitor->getMarkingDeque()->first().rawObjectPointer(), nullptr);
-    visitor->getMarkingDeque()->clear();
-    visitor->getVerifierDeque()->clear();
-}
-
-} // namespace blink

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

@@ -391,16 +391,12 @@ void V8GCController::gcEpilogue(v8::Isolate* isolate, v8::GCType type, v8::GCCal
     TRACE_EVENT_INSTANT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "UpdateCounters", TRACE_EVENT_SCOPE_THREAD, "data", InspectorUpdateCountersEvent::data());
 }
 
-void V8GCController::collectGarbage(v8::Isolate* isolate, bool onlyMinorGC)
+void V8GCController::collectGarbage(v8::Isolate* isolate)
 {
     v8::HandleScope handleScope(isolate);
     RefPtr<ScriptState> scriptState = ScriptState::create(v8::Context::New(isolate), DOMWrapperWorld::create(isolate));
     ScriptState::Scope scope(scriptState.get());
-    StringBuilder builder;
-    builder.append("if (gc) gc(");
-    builder.append(onlyMinorGC ? "true" : "false");
-    builder.append(")");
-    V8ScriptRunner::compileAndRunInternalScript(v8String(isolate, builder.toString()), isolate);
+    V8ScriptRunner::compileAndRunInternalScript(v8String(isolate, "if (gc) gc();"), isolate);
     scriptState->disposePerContextData();
 }
 

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

@@ -47,7 +47,7 @@ public:
     static void gcPrologue(v8::Isolate*, v8::GCType, v8::GCCallbackFlags);
     static void gcEpilogue(v8::Isolate*, v8::GCType, v8::GCCallbackFlags);
 
-    static void collectGarbage(v8::Isolate*, bool onlyMinorGC = false);
+    static void collectGarbage(v8::Isolate*);
     // You should use collectAllGarbageForTesting() when you want to collect all
     // V8 & Blink objects. It runs multiple V8 GCs to collect references
     // that cross the binding boundary. collectAllGarbage() also runs multipe

third_party/WebKit/Source/core/BUILD.gn

@@ -213,8 +213,6 @@ source_set("testing") {
   sources = [
     "$blink_core_output_dir/testing/InternalSettingsGenerated.cpp",
     "$blink_core_output_dir/testing/InternalSettingsGenerated.h",
-    "testing/DeathAwareScriptWrappable.cpp",
-    "testing/DeathAwareScriptWrappable.h",
     "testing/DictionaryTest.cpp",
     "testing/DictionaryTest.h",
     "testing/DummyPageHolder.cpp",

third_party/WebKit/Source/core/core_idl_files.gni

@@ -574,7 +574,6 @@ core_testing_dictionary_idl_files =
 
 webcore_testing_idl_files =
     get_path_info([
-                    "testing/DeathAwareScriptWrappable.idl",
                     "testing/DictionaryTest.idl",
                     "testing/GCObservation.idl",
                     "testing/GarbageCollectedScriptWrappable.idl",

third_party/WebKit/Source/core/testing/DeathAwareScriptWrappable.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 "core/testing/DeathAwareScriptWrappable.h"
-
-namespace blink {
-
-DeathAwareScriptWrappable* DeathAwareScriptWrappable::s_instance;
-bool DeathAwareScriptWrappable::s_hasDied;
-
-} // namespace blink

third_party/WebKit/Source/core/testing/DeathAwareScriptWrappable.h

-// Copyright 2016 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.
-
-#ifndef DeathAwareScriptWrappable_h
-#define DeathAwareScriptWrappable_h
-
-#include "bindings/core/v8/ScriptWrappable.h"
-#include "platform/heap/Heap.h"
-#include "wtf/text/WTFString.h"
-#include <signal.h>
-
-namespace blink {
-
-class DeathAwareScriptWrappable : public GarbageCollectedFinalized<DeathAwareScriptWrappable>, public ScriptWrappable {
-    DEFINE_WRAPPERTYPEINFO();
-    static DeathAwareScriptWrappable* s_instance;
-    static bool s_hasDied;
-
-public:
-    virtual ~DeathAwareScriptWrappable()
-    {
-        if (this == s_instance) {
-            s_hasDied = true;
-        }
-    }
-
-    static DeathAwareScriptWrappable* create()
-    {
-        return new DeathAwareScriptWrappable();
-    }
-
-    static bool hasDied() { return s_hasDied; }
-    static void observeDeathsOf(DeathAwareScriptWrappable* instance)
-    {
-        s_hasDied = false;
-        s_instance = instance;
-    }
-
-    DEFINE_INLINE_VIRTUAL_TRACE() {}
-    DEFINE_INLINE_VIRTUAL_TRACE_WRAPPERS() {}
-};
-
-} // namespace blink
-
-#endif // DeathAwareScriptWrappable_h

third_party/WebKit/Source/core/testing/DeathAwareScriptWrappable.idl

-// Copyright 2016 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.
-
-interface DeathAwareScriptWrappable {
-};

third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in

@@ -228,7 +228,7 @@ TimerThrottlingForHiddenFrames status=stable
 // Chromium sets this conditionally (eg. based on the presence of a
 // touchscreen) in ApplyWebPreferences.
 Touch status=stable
-TraceWrappables status=test
+TraceWrappables
 TrustedEventsDefaultAction status=stable
 UnsafeES3APIs
 UnsandboxedAuxiliary status=stable

third_party/WebKit/Source/platform/heap/ThreadState.cpp

@@ -1007,7 +1007,7 @@ void ThreadState::makeConsistentForMutator()
 
 void ThreadState::preGC()
 {
-    if (RuntimeEnabledFeatures::traceWrappablesEnabled() && m_isolate && m_performCleanup)
+    if (RuntimeEnabledFeatures::traceWrappablesEnabled() && m_performCleanup)
         m_performCleanup(m_isolate);
 
     ASSERT(!isInGC());