Attachment 343653 Details for Bug 186878 – patch

[patch] patch

a-backup.diff (text/plain), 30.96 KB, created by Saam Barati on 2018-06-26 16:01:55 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-06-26 16:01:55 PDT

Size: 30.96 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 233226)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,59 @@
>+2018-06-26  Saam Barati  <sbarati@apple.com>
>+
>+        JSImmutableButterfly can't be allocated from a subspace with HeapCell::Kind::Auxiliary
>+        https://bugs.webkit.org/show_bug.cgi?id=186878
>+        <rdar://problem/40568659>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch fixes a bug in our JSImmutableButterfly implementation uncovered by
>+        our stress GC bots. Before this patch, JSImmutableButterfly was allocated
>+        with HeapCell::Kind::Auxiliary. This is wrong. Things that are JSCells can't
>+        be allocated from HeapCell::Kind::Auxiliary. This patch adds a new HeapCell::Kind
>+        called JSCellWithInteriorPointers. It behaves like JSCell in all ways, except
>+        conservative scan knows to treat it like a butterfly in when we we may be
>+        pointing into the middle of it.
>+        
>+        The way we were crashing on the stress GC bots is that our conservative marking
>+        won't do cell visiting for things that are Auxiliary. This meant that if the
>+        stack were the only thing pointing to a JSImmutableButterfly when a GC took place,
>+        that JSImmutableButterfly would not be visited. This is now fixed.
>+
>+        * bytecompiler/NodesCodegen.cpp:
>+        (JSC::ArrayNode::emitBytecode):
>+        * debugger/Debugger.cpp:
>+        * heap/ConservativeRoots.cpp:
>+        (JSC::ConservativeRoots::genericAddPointer):
>+        * heap/Heap.cpp:
>+        (JSC::GatherHeapSnapshotData::operator() const):
>+        (JSC::RemoveDeadHeapSnapshotNodes::operator() const):
>+        (JSC::Heap::globalObjectCount):
>+        (JSC::Heap::objectTypeCounts):
>+        (JSC::Heap::deleteAllCodeBlocks):
>+        * heap/HeapCell.cpp:
>+        (WTF::printInternal):
>+        * heap/HeapCell.h:
>+        (JSC::isJSCellKind):
>+        (JSC::hasInteriorPointers):
>+        * heap/HeapUtil.h:
>+        (JSC::HeapUtil::findGCObjectPointersForMarking):
>+        (JSC::HeapUtil::isPointerGCObjectJSCell):
>+        * heap/MarkedBlock.cpp:
>+        (JSC::MarkedBlock::Handle::didAddToDirectory):
>+        * heap/SlotVisitor.cpp:
>+        (JSC::SlotVisitor::appendJSCellOrAuxiliary):
>+        * runtime/JSGlobalObject.cpp:
>+        * runtime/JSImmutableButterfly.h:
>+        (JSC::JSImmutableButterfly::subspaceFor):
>+        * runtime/VM.cpp:
>+        (JSC::VM::VM):
>+        * runtime/VM.h:
>+        * tools/CellProfile.h:
>+        (JSC::CellProfile::CellProfile):
>+        (JSC::CellProfile::isJSCell const):
>+        * tools/HeapVerifier.cpp:
>+        (JSC::HeapVerifier::validateCell):
>+
> 2018-06-26  Mark Lam  <mark.lam@apple.com>
> 
>         ASSERTION FAILED: length > butterfly->vectorLength() in JSObject::ensureLengthSlow().
>Index: Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp	(revision 233216)
>+++ Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp	(working copy)
>@@ -406,6 +406,7 @@ RegisterID* ArrayNode::emitBytecode(Byte
>     auto newArray = [&] (RegisterID* dst, ElementNode* elements, unsigned length, bool hadVariableExpression) {
>         if (length && !hadVariableExpression) {
>             recommendedIndexingType |= CopyOnWrite;
>+            ASSERT(generator.vm()->heap.isDeferred()); // We run bytecode generator under a DeferGC. If we stopped doing that, we'd need to put a DeferGC here as we filled in these slots.
>             auto* array = JSImmutableButterfly::create(*generator.vm(), recommendedIndexingType, length);
>             unsigned index = 0;
>             for (ElementNode* element = elements; index < length; element = element->next()) {
>Index: Source/JavaScriptCore/debugger/Debugger.cpp
>===================================================================
>--- Source/JavaScriptCore/debugger/Debugger.cpp	(revision 233216)
>+++ Source/JavaScriptCore/debugger/Debugger.cpp	(working copy)
>@@ -51,7 +51,7 @@ struct GatherSourceProviders : public Ma
> 
>     IterationStatus operator()(HeapCell* heapCell, HeapCell::Kind kind) const
>     {
>-        if (kind != HeapCell::JSCell)
>+        if (!isJSCellKind(kind))
>             return IterationStatus::Continue;
>         
>         JSCell* cell = static_cast<JSCell*>(heapCell);
>Index: Source/JavaScriptCore/heap/ConservativeRoots.cpp
>===================================================================
>--- Source/JavaScriptCore/heap/ConservativeRoots.cpp	(revision 233216)
>+++ Source/JavaScriptCore/heap/ConservativeRoots.cpp	(working copy)
>@@ -73,7 +73,7 @@ inline void ConservativeRoots::genericAd
>     HeapUtil::findGCObjectPointersForMarking(
>         m_heap, markingVersion, newlyAllocatedVersion, filter, p,
>         [&] (void* p, HeapCell::Kind cellKind) {
>-            if (cellKind == HeapCell::JSCell)
>+            if (isJSCellKind(cellKind))
>                 markHook.markKnownJSCell(static_cast<JSCell*>(p));
>             
>             if (m_size == m_capacity)
>Index: Source/JavaScriptCore/heap/Heap.cpp
>===================================================================
>--- Source/JavaScriptCore/heap/Heap.cpp	(revision 233216)
>+++ Source/JavaScriptCore/heap/Heap.cpp	(working copy)
>@@ -720,7 +720,7 @@ struct GatherHeapSnapshotData : MarkedBl
> 
>     IterationStatus operator()(HeapCell* heapCell, HeapCell::Kind kind) const
>     {
>-        if (kind == HeapCell::JSCell) {
>+        if (isJSCellKind(kind)) {
>             JSCell* cell = static_cast<JSCell*>(heapCell);
>             cell->methodTable()->heapSnapshot(cell, m_builder);
>         }
>@@ -747,7 +747,7 @@ struct RemoveDeadHeapSnapshotNodes : Mar
> 
>     IterationStatus operator()(HeapCell* cell, HeapCell::Kind kind) const
>     {
>-        if (kind == HeapCell::JSCell)
>+        if (isJSCellKind(kind))
>             m_snapshot.sweepCell(static_cast<JSCell*>(cell));
>         return IterationStatus::Continue;
>     }
>@@ -836,7 +836,7 @@ size_t Heap::globalObjectCount()
>     m_objectSpace.forEachLiveCell(
>         iterationScope,
>         [&] (HeapCell* heapCell, HeapCell::Kind kind) -> IterationStatus {
>-            if (kind != HeapCell::JSCell)
>+            if (!isJSCellKind(kind))
>                 return IterationStatus::Continue;
>             JSCell* cell = static_cast<JSCell*>(heapCell);
>             if (cell->isObject() && asObject(cell)->isGlobalObject())
>@@ -873,7 +873,7 @@ std::unique_ptr<TypeCountSet> Heap::obje
>     m_objectSpace.forEachLiveCell(
>         iterationScope,
>         [&] (HeapCell* cell, HeapCell::Kind kind) -> IterationStatus {
>-            if (kind == HeapCell::JSCell)
>+            if (isJSCellKind(kind))
>                 recordType(*vm(), *result, static_cast<JSCell*>(cell));
>             return IterationStatus::Continue;
>         });
>@@ -915,7 +915,7 @@ void Heap::deleteAllCodeBlocks(DeleteAll
>         // it uses a callee check, but then it will call into dead code.
>         HeapIterationScope heapIterationScope(*this);
>         vm.webAssemblyCodeBlockSpace.forEachLiveCell([&] (HeapCell* cell, HeapCell::Kind kind) {
>-            ASSERT_UNUSED(kind, kind == HeapCell::Kind::JSCell);
>+            ASSERT_UNUSED(kind, kind == HeapCell::JSCell);
>             JSWebAssemblyCodeBlock* codeBlock = static_cast<JSWebAssemblyCodeBlock*>(cell);
>             codeBlock->clearJSCallICs(vm);
>         });
>Index: Source/JavaScriptCore/heap/HeapCell.cpp
>===================================================================
>--- Source/JavaScriptCore/heap/HeapCell.cpp	(revision 233216)
>+++ Source/JavaScriptCore/heap/HeapCell.cpp	(working copy)
>@@ -60,6 +60,9 @@ void printInternal(PrintStream& out, Hea
>     case HeapCell::JSCell:
>         out.print("JSCell");
>         return;
>+    case HeapCell::JSCellWithInteriorPointers:
>+        out.print("JSCellWithInteriorPointers");
>+        return;
>     case HeapCell::Auxiliary:
>         out.print("Auxiliary");
>         return;
>Index: Source/JavaScriptCore/heap/HeapCell.h
>===================================================================
>--- Source/JavaScriptCore/heap/HeapCell.h	(revision 233216)
>+++ Source/JavaScriptCore/heap/HeapCell.h	(working copy)
>@@ -43,6 +43,7 @@ class HeapCell {
> public:
>     enum Kind : int8_t {
>         JSCell,
>+        JSCellWithInteriorPointers,
>         Auxiliary
>     };
>     
>@@ -86,6 +87,16 @@ public:
> #endif
> };
> 
>+inline bool isJSCellKind(HeapCell::Kind kind)
>+{
>+    return kind == HeapCell::JSCell || kind == HeapCell::JSCellWithInteriorPointers;
>+}
>+
>+inline bool hasInteriorPointers(HeapCell::Kind kind)
>+{
>+    return kind == HeapCell::Auxiliary || kind == HeapCell::JSCellWithInteriorPointers;
>+}
>+
> } // namespace JSC
> 
> namespace WTF {
>Index: Source/JavaScriptCore/heap/HeapUtil.h
>===================================================================
>--- Source/JavaScriptCore/heap/HeapUtil.h	(revision 233216)
>+++ Source/JavaScriptCore/heap/HeapUtil.h	(working copy)
>@@ -88,7 +88,7 @@ public:
>             MarkedBlock* previousCandidate = MarkedBlock::blockFor(previousPointer);
>             if (!filter.ruleOut(bitwise_cast<Bits>(previousCandidate))
>                 && set.contains(previousCandidate)
>-                && previousCandidate->handle().cellKind() == HeapCell::Auxiliary) {
>+                && hasInteriorPointers(previousCandidate->handle().cellKind())) {
>                 previousPointer = static_cast<char*>(previousCandidate->handle().cellAlign(previousPointer));
>                 if (previousCandidate->handle().isLiveCell(markingVersion, newlyAllocatedVersion, isMarking, previousPointer))
>                     func(previousPointer, previousCandidate->handle().cellKind());
>@@ -110,12 +110,11 @@ public:
>                 func(pointer, cellKind);
>         };
>     
>-        if (candidate->handle().cellKind() == HeapCell::JSCell) {
>-            if (!MarkedBlock::isAtomAligned(pointer))
>+        if (isJSCellKind(cellKind)) {
>+            if (MarkedBlock::isAtomAligned(pointer))
>+                tryPointer(pointer);
>+            if (!hasInteriorPointers(cellKind))
>                 return;
>-        
>-            tryPointer(pointer);
>-            return;
>         }
>     
>         // A butterfly could point into the middle of an object.
>@@ -144,14 +143,14 @@ public:
>                 if (result) {
>                     if (result > largeAllocations.begin()
>                         && result[-1]->cell() == pointer
>-                        && result[-1]->attributes().cellKind == HeapCell::JSCell)
>+                        && isJSCellKind(result[-1]->attributes().cellKind))
>                         return true;
>                     if (result[0]->cell() == pointer
>-                        && result[0]->attributes().cellKind == HeapCell::JSCell)
>+                        && isJSCellKind(result[0]->attributes().cellKind))
>                         return true;
>                     if (result + 1 < largeAllocations.end()
>                         && result[1]->cell() == pointer
>-                        && result[1]->attributes().cellKind == HeapCell::JSCell)
>+                        && isJSCellKind(result[1]->attributes().cellKind))
>                         return true;
>                 }
>             }
>Index: Source/JavaScriptCore/heap/MarkedBlock.cpp
>===================================================================
>--- Source/JavaScriptCore/heap/MarkedBlock.cpp	(revision 233216)
>+++ Source/JavaScriptCore/heap/MarkedBlock.cpp	(working copy)
>@@ -351,7 +351,7 @@ void MarkedBlock::Handle::didAddToDirect
>     
>     m_attributes = directory->attributes();
> 
>-    if (m_attributes.cellKind != HeapCell::JSCell)
>+    if (!isJSCellKind(m_attributes.cellKind))
>         RELEASE_ASSERT(m_attributes.destruction == DoesNotNeedDestruction);
>     
>     double markCountBias = -(Options::minMarkedBlockUtilization() * cellsPerBlock());
>Index: Source/JavaScriptCore/heap/SlotVisitor.cpp
>===================================================================
>--- Source/JavaScriptCore/heap/SlotVisitor.cpp	(revision 233216)
>+++ Source/JavaScriptCore/heap/SlotVisitor.cpp	(working copy)
>@@ -196,14 +196,15 @@ void SlotVisitor::appendJSCellOrAuxiliar
>     
>     // In debug mode, we validate before marking since this makes it clearer what the problem
>     // was. It's also slower, so we don't do it normally.
>-    if (!ASSERT_DISABLED && heapCell->cellKind() == HeapCell::JSCell)
>+    if (!ASSERT_DISABLED && isJSCellKind(heapCell->cellKind()))
>         validateCell(static_cast<JSCell*>(heapCell));
>     
>     if (Heap::testAndSetMarked(m_markingVersion, heapCell))
>         return;
>     
>     switch (heapCell->cellKind()) {
>-    case HeapCell::JSCell: {
>+    case HeapCell::JSCell:
>+    case HeapCell::JSCellWithInteriorPointers: {
>         // We have ample budget to perform validation here.
>     
>         JSCell* jsCell = static_cast<JSCell*>(heapCell);
>Index: Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/JSGlobalObject.cpp	(revision 233216)
>+++ Source/JavaScriptCore/runtime/JSGlobalObject.cpp	(working copy)
>@@ -1239,7 +1239,7 @@ inline void ObjectsWithBrokenIndexingFin
> 
> IterationStatus ObjectsWithBrokenIndexingFinder::operator()(HeapCell* cell, HeapCell::Kind kind) const
> {
>-    if (kind == HeapCell::JSCell) {
>+    if (isJSCellKind(kind)) {
>         // FIXME: This const_cast exists because this isn't a C++ lambda.
>         // https://bugs.webkit.org/show_bug.cgi?id=159644
>         const_cast<ObjectsWithBrokenIndexingFinder*>(this)->visit(static_cast<JSCell*>(cell));
>Index: Source/JavaScriptCore/runtime/JSImmutableButterfly.h
>===================================================================
>--- Source/JavaScriptCore/runtime/JSImmutableButterfly.h	(revision 233216)
>+++ Source/JavaScriptCore/runtime/JSImmutableButterfly.h	(working copy)
>@@ -89,7 +89,7 @@ public:
>     static CompleteSubspace* subspaceFor(VM& vm)
>     {
>         // We allocate out of the JSValue gigacage as other code expects all butterflies to live there.
>-        return &vm.jsValueGigacageAuxiliarySpace;
>+        return &vm.immutableButterflyJSValueGigacageAuxiliarySpace;
>     }
> 
>     // Only call this if you just allocated this butterfly.
>Index: Source/JavaScriptCore/runtime/VM.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/VM.cpp	(revision 233216)
>+++ Source/JavaScriptCore/runtime/VM.cpp	(working copy)
>@@ -254,6 +254,7 @@ VM::VM(VMType vmType, HeapType heapType)
>     , jsValueGigacageAllocator(std::make_unique<GigacageAlignedMemoryAllocator>(Gigacage::JSValue))
>     , auxiliaryHeapCellType(std::make_unique<HeapCellType>(CellAttributes(DoesNotNeedDestruction, HeapCell::Auxiliary)))
>     , cellJSValueOOBHeapCellType(std::make_unique<HeapCellType>(CellAttributes(DoesNotNeedDestruction, HeapCell::JSCell)))
>+    , immutableButterflyHeapCellType(std::make_unique<HeapCellType>(CellAttributes(DoesNotNeedDestruction, HeapCell::JSCellWithInteriorPointers)))
>     , cellDangerousBitsHeapCellType(std::make_unique<HeapCellType>(CellAttributes(DoesNotNeedDestruction, HeapCell::JSCell)))
>     , destructibleCellHeapCellType(std::make_unique<HeapCellType>(CellAttributes(NeedsDestruction, HeapCell::JSCell)))
>     , stringHeapCellType(std::make_unique<JSStringHeapCellType>())
>@@ -264,6 +265,7 @@ VM::VM(VMType vmType, HeapType heapType)
> #endif
>     , primitiveGigacageAuxiliarySpace("Primitive Gigacage Auxiliary", heap, auxiliaryHeapCellType.get(), primitiveGigacageAllocator.get())
>     , jsValueGigacageAuxiliarySpace("JSValue Gigacage Auxiliary", heap, auxiliaryHeapCellType.get(), jsValueGigacageAllocator.get())
>+    , immutableButterflyJSValueGigacageAuxiliarySpace("ImmutableButterfly Gigacage JSCellWithInteriorPointers", heap, immutableButterflyHeapCellType.get(), jsValueGigacageAllocator.get())
>     , cellJSValueOOBSpace("JSCell JSValueOOB", heap, cellJSValueOOBHeapCellType.get(), fastMallocAllocator.get())
>     , cellDangerousBitsSpace("JSCell DangerousBits", heap, cellDangerousBitsHeapCellType.get(), fastMallocAllocator.get())
>     , jsValueGigacageCellSpace("JSValue Gigacage JSCell", heap, cellJSValueOOBHeapCellType.get(), jsValueGigacageAllocator.get())
>Index: Source/JavaScriptCore/runtime/VM.h
>===================================================================
>--- Source/JavaScriptCore/runtime/VM.h	(revision 233216)
>+++ Source/JavaScriptCore/runtime/VM.h	(working copy)
>@@ -306,6 +306,7 @@ public:
> 
>     std::unique_ptr<HeapCellType> auxiliaryHeapCellType;
>     std::unique_ptr<HeapCellType> cellJSValueOOBHeapCellType;
>+    std::unique_ptr<HeapCellType> immutableButterflyHeapCellType;
>     std::unique_ptr<HeapCellType> cellDangerousBitsHeapCellType;
>     std::unique_ptr<HeapCellType> destructibleCellHeapCellType;
>     std::unique_ptr<JSStringHeapCellType> stringHeapCellType;
>@@ -317,6 +318,7 @@ public:
>     
>     CompleteSubspace primitiveGigacageAuxiliarySpace; // Typed arrays, strings, bitvectors, etc go here.
>     CompleteSubspace jsValueGigacageAuxiliarySpace; // Butterflies, arrays of JSValues, etc go here.
>+    CompleteSubspace immutableButterflyJSValueGigacageAuxiliarySpace; // JSImmutableButterfly goes here.
> 
>     // We make cross-cutting assumptions about typed arrays being in the primitive Gigacage and butterflies
>     // being in the JSValue gigacage. For some types, it's super obvious where they should go, and so we
>Index: Source/JavaScriptCore/tools/CellProfile.h
>===================================================================
>--- Source/JavaScriptCore/tools/CellProfile.h	(revision 233216)
>+++ Source/JavaScriptCore/tools/CellProfile.h	(working copy)
>@@ -45,7 +45,7 @@ struct CellProfile {
>         , m_liveness(liveness)
>         , m_timestamp(MonotonicTime::now())
>     {
>-        if (m_kind == HeapCell::JSCell && m_liveness != Dead)
>+        if (isJSCellKind(m_kind) && m_liveness != Dead)
>             m_className = jsCell()->structure()->classInfo()->className;
>     }
> 
>@@ -65,7 +65,7 @@ struct CellProfile {
>         return static_cast<JSCell*>(m_cell);
>     }
> 
>-    bool isJSCell() const { return m_kind == HeapCell::JSCell; }
>+    bool isJSCell() const { return isJSCellKind(m_kind); }
>     
>     HeapCell::Kind kind() const { return m_kind; }
> 
>Index: Source/JavaScriptCore/tools/HeapVerifier.cpp
>===================================================================
>--- Source/JavaScriptCore/tools/HeapVerifier.cpp	(revision 233216)
>+++ Source/JavaScriptCore/tools/HeapVerifier.cpp	(working copy)
>@@ -188,7 +188,7 @@ bool HeapVerifier::validateCell(HeapCell
>         return false;
>     }
> 
>-    if (cell->cellKind() != HeapCell::JSCell)
>+    if (!isJSCellKind(cell->cellKind()))
>         return true; // Nothing more to validate.
> 
>     JSCell* jsCell = static_cast<JSCell*>(cell);
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 233216)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,24 @@
>+2018-06-26  Saam Barati  <sbarati@apple.com>
>+
>+        JSImmutableButterfly can't be allocated from a subspace with HeapCell::Kind::Auxiliary
>+        https://bugs.webkit.org/show_bug.cgi?id=186878
>+        <rdar://problem/40568659>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Make these test not susceptible to conservative scan leaks by ensuring at least
>+        one object gets collected when we allocate many of them. Before, these were just
>+        testing that a fixed number of objects were collected.
>+
>+        * editing/selection/navigation-clears-editor-state-expected.txt:
>+        * editing/selection/navigation-clears-editor-state.html:
>+        * fast/dom/reference-cycle-leaks.html:
>+        * fast/misc/resources/test-observegc.js:
>+        * fast/misc/test-observegc-expected.txt:
>+        * platform/mac-wk2/plugins/refcount-leaks-expected.txt:
>+        * plugins/refcount-leaks-expected.txt:
>+        * plugins/refcount-leaks.html:
>+
> 2018-06-26  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r233184.
>Index: LayoutTests/editing/selection/navigation-clears-editor-state-expected.txt
>===================================================================
>--- LayoutTests/editing/selection/navigation-clears-editor-state-expected.txt	(revision 233216)
>+++ LayoutTests/editing/selection/navigation-clears-editor-state-expected.txt	(working copy)
>@@ -3,10 +3,7 @@ This tests navigating away from a docume
> On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
> 
> 
>-iframe = appendIframe()
>-PASS internals.numberOfLiveDocuments() is initialDocumentCount + 1
>-iframe.src = "resources/select-iframe-focusin-document-crash-frame.html"
>-PASS internals.numberOfLiveDocuments() is initialDocumentCount + 1
>+PASS freed more than 60 documents
> PASS successfullyParsed is true
> 
> TEST COMPLETE
>Index: LayoutTests/editing/selection/navigation-clears-editor-state.html
>===================================================================
>--- LayoutTests/editing/selection/navigation-clears-editor-state.html	(revision 233216)
>+++ LayoutTests/editing/selection/navigation-clears-editor-state.html	(working copy)
>@@ -35,21 +35,36 @@ async function runTest()
> {
>     initialDocumentCount = internals.numberOfLiveDocuments();
> 
>-    evalAndLog('iframe = appendIframe()');
>+    let frames = [];
>+    const count = 200;
>+    for (let i = 0; i < count; ++i)
>+        frames.push(appendIframe());
> 
>     await wait(0); // Make sure the transient document created by inserting an iframe is removed.
>     GCController.collect();
> 
>-    shouldBe('internals.numberOfLiveDocuments()', 'initialDocumentCount + 1');
>-    setEditorStates(iframe);
>+    for (let frame of frames)
>+        setEditorStates(frame);
> 
>     await wait(0); // Wait for UI update timer to fire.
> 
>-    evalAndLog('iframe.src = "resources/select-iframe-focusin-document-crash-frame.html"');
>-    iframe.onload = () => {
>-        GCController.collect();
>-        shouldBe('internals.numberOfLiveDocuments()', 'initialDocumentCount + 1');
>-        finishJSTest();                
>+    let resolved = 0;
>+    for (let frame of frames) {
>+        frame.src = "resources/select-iframe-focusin-document-crash-frame.html";
>+        frame.onload = () => {
>+            ++resolved;
>+            if (resolved !== count)
>+                return;
>+            let before = internals.numberOfLiveDocuments();
>+            GCController.collect();
>+            let after = internals.numberOfLiveDocuments();
>+            let delta = before - after;
>+            if (delta > 0.3 * count)
>+                debug("PASS freed more than 60 documents");
>+            else
>+                debug("FAIL freed fewer than 60 documents");
>+            finishJSTest();                
>+        };
>     }
> }
> 
>@@ -57,7 +72,7 @@ if (!window.GCController || !window.inte
>     debug('This test requires GCController and internals');
> } else {
>     if (window.testRunner)
>-        setTimeout(() => testRunner.notifyDone(), 3000);
>+        setTimeout(() => testRunner.notifyDone(), 5000);
>     // Clear out any lingering documents from the previous tests.
>     GCController.collect();
>     GCController.collect();
>Index: LayoutTests/fast/dom/reference-cycle-leaks.html
>===================================================================
>--- LayoutTests/fast/dom/reference-cycle-leaks.html	(revision 233216)
>+++ LayoutTests/fast/dom/reference-cycle-leaks.html	(working copy)
>@@ -7,7 +7,7 @@ description('Tests for leaks caused by r
> function checkForNodeLeaks(testFunction, underlyingClass)
> {
>     // Bump this number as high as we need to, to get reproducible results.
>-    const repetitions = 20;
>+    const repetitions = 40;
> 
>     gc();
>     const beforeCount = internals.numberOfLiveNodes();
>Index: LayoutTests/fast/misc/test-observegc-expected.txt
>===================================================================
>--- LayoutTests/fast/misc/test-observegc-expected.txt	(revision 233216)
>+++ LayoutTests/fast/misc/test-observegc-expected.txt	(working copy)
>@@ -3,7 +3,7 @@ Ensures that window.internals.observegc 
> On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
> 
> 
>-PASS observer.wasCollected is true
>+PASS anyCollected is true
> PASS successfullyParsed is true
> 
> TEST COMPLETE
>Index: LayoutTests/fast/misc/resources/test-observegc.js
>===================================================================
>--- LayoutTests/fast/misc/resources/test-observegc.js	(revision 233216)
>+++ LayoutTests/fast/misc/resources/test-observegc.js	(working copy)
>@@ -1,9 +1,19 @@
> description("Ensures that window.internals.observegc works as expected");
> 
>-var testObject = { testProperty : "testValue" };
>+var observers = [];
>+for (let i = 0; i < 1000; ++i) {
>+    let testObject = { testProperty : "testValue" };
>+    let observer = internals.observeGC(testObject);
>+    observers.push(observer);
>+    testObject = null;
>+}
> 
>-var observer = internals.observeGC(testObject);
>-testObject = null;
> gc();
> 
>-shouldBe('observer.wasCollected', 'true');
>+var anyCollected = false;
>+for (let observer of observers) {
>+    if (observer.wasCollected)
>+        anyCollected = true;
>+}
>+
>+shouldBe('anyCollected', 'true');
>Index: LayoutTests/platform/mac-wk2/plugins/refcount-leaks-expected.txt
>===================================================================
>--- LayoutTests/platform/mac-wk2/plugins/refcount-leaks-expected.txt	(revision 233216)
>+++ LayoutTests/platform/mac-wk2/plugins/refcount-leaks-expected.txt	(working copy)
>@@ -1,12 +1,8 @@
>+PASS countAfterCreate === countOrig + 50 is true
>+FAIL countAfterGC < countAfterCreate should be true. Was false.
>+PASS refAfterGet === refOrig + 50 is true
>+FAIL refAfterGetGC < refAfterGet should be true. Was false.
>+PASS refAfterPass === refBeforePass + 50 is true
>+FAIL refAfterPassAndGC < refAfterPass should be true. Was false.
> Test that we can get an NPObject returned through a method on an NPAPI Object.
>-Prints "SUCCESS" on success, "FAILURE" on failure.  
> 
>---- num test objects:
>-countAfterCreate == countOrig + 3? PASS
>-countOrig == countAfterGC? FAIL
>-
>---- refcount on plug.testObject:
>-originally: 2
>-after GC: 5
>-after passing: 8
>-FAILURE
>Index: LayoutTests/plugins/refcount-leaks-expected.txt
>===================================================================
>--- LayoutTests/plugins/refcount-leaks-expected.txt	(revision 233216)
>+++ LayoutTests/plugins/refcount-leaks-expected.txt	(working copy)
>@@ -1,12 +1,8 @@
>+PASS countAfterCreate === countOrig + 50 is true
>+PASS countAfterGC < countAfterCreate is true
>+PASS refAfterGet === refOrig + 50 is true
>+PASS refAfterGetGC < refAfterGet is true
>+PASS refAfterPass === refBeforePass + 50 is true
>+PASS refAfterPassAndGC < refAfterPass is true
> Test that we can get an NPObject returned through a method on an NPAPI Object.
>-Prints "SUCCESS" on success, "FAILURE" on failure.  
> 
>---- num test objects:
>-countAfterCreate == countOrig + 3? PASS
>-countOrig == countAfterGC? PASS
>-
>---- refcount on plug.testObject:
>-originally: 2
>-after GC: 2
>-after passing: 2
>-SUCCESS
>Index: LayoutTests/plugins/refcount-leaks.html
>===================================================================
>--- LayoutTests/plugins/refcount-leaks.html	(revision 233216)
>+++ LayoutTests/plugins/refcount-leaks.html	(working copy)
>@@ -1,83 +1,74 @@
>+<head>
>+<script src="../resources/js-test-pre.js"></script>
>+</head>
>+
> <script>
>-  function noop(x) {
>-  }
>+function noop(x) {
>+}
> 
>-  function doGC() {
>-    if (window.GCController) {
>-      // GC twice to make sure everything is cleaned up.
>-      for (var i = 0; i < 2; i++) {
>-        window.GCController.collect();
>-      }
>+function doGC() {
>+    // GC twice to make sure everything is cleaned up.
>+    for (var i = 0; i < 2; i++) {
>+        gc();
>     }
>-  }
>-
>-  function runtest() {
>-    if (window.testRunner)
>-      testRunner.dumpAsText();
>+}
> 
>+var countOrig;
>+var countAfterCreate;
>+var countAfterGC;
>+var testObj;
>+var refOrig;
>+var refAfterGet;
>+var refAfterGetGC;
>+var refBeforePass;
>+var refAfterPass;
>+var refAfterPassAndGC;
> 
>-    var output = document.getElementById("output");
>-    output.innerHTML = "";
>+function runtest() {
>+    if (window.testRunner)
>+        testRunner.dumpAsText();
> 
>     // Test that objects are deleted after their JS references are released.
>-    var countOrig = plug.testObjectCount;
>-    o1 = plug.testCreateTestObject();
>-    o2 = plug.testCreateTestObject();
>-    o3 = plug.testCreateTestObject();
>-    var countAfterCreate = plug.testObjectCount;
>-    o1 = o2 = o3 = null;
>+    countOrig = plug.testObjectCount;
>+    let x;
>+    for (let i = 0; i < 50; ++i)
>+        x = plug.testCreateTestObject();
>+    countAfterCreate = plug.testObjectCount;
>+    x = null;
>     doGC();
>-    var countAfterGC = plug.testObjectCount;
>-
>-    output.innerHTML += "--- num test objects:<br>";
>-    output.innerHTML += "countAfterCreate == countOrig + 3? "
>-        + ((countAfterCreate == countOrig + 3) ? "PASS" : "FAIL")
>-        + "<br>";
>-    output.innerHTML += "countOrig == countAfterGC? "
>-        + ((countOrig == countAfterGC) ? "PASS" : "FAIL")
>-        + "<br>";
>-    output.innerHTML += "<br>";
>+    countAfterGC = plug.testObjectCount;
>+    shouldBe('countAfterCreate === countOrig + 50', 'true');
>+    shouldBe('countAfterGC < countAfterCreate', 'true');
> 
>     // Test that the object refcount returns to normal after JS references
>     // are released.
>-    var testObj = plug.testObject;
>-    var refOrig = testObj.refCount;
>-    var o1 = plug.testObject;
>-    var o2 = plug.testObject;
>-    var o3 = plug.testObject;
>-    var refAfterGet = testObj.refCount;
>-    o1 = o2 = o3 = null;
>+    testObj = plug.testObject;
>+    refOrig = testObj.refCount;
>+    for (let i = 0; i < 50; ++i)
>+        plug.testObject;
>+    refAfterGet = testObj.refCount;
>     doGC();
>-    var refAfterGetGC = testObj.refCount;
>+    refAfterGetGC = testObj.refCount;
>+    shouldBe('refAfterGet === refOrig + 50', 'true');
>+    shouldBe('refAfterGetGC < refAfterGet', 'true');
> 
>     // Test that calling NPN_Invoke with our object as a parameter returns
>     // our refcount to normal (may require a GC).
>-    plug.testPassTestObject("noop", testObj);
>-    plug.testPassTestObject("noop", testObj);
>-    plug.testPassTestObject("noop", testObj);
>+    refBeforePass = testObj.refCount;
>+    for (let i = 0; i < 50; ++i)
>+        plug.testPassTestObject("noop", testObj);
>+    refAfterPass = testObj.refCount;
>     doGC();
>-    var refAfterPass = testObj.refCount;
>-
>-    output.innerHTML += "--- refcount on plug.testObject:<br>";
>-    output.innerHTML += "originally: " + refOrig + "<br>";
>-    output.innerHTML += "after GC: " + refAfterGetGC + "<br>";
>-    output.innerHTML += "after passing: " + refAfterPass + "<br>";
>-
>-    var success = (countAfterGC == countOrig) && (refAfterPass == refOrig);
>-    output.innerHTML += (success ? "SUCCESS" : "FAILURE");
>-  }
>+    refAfterPassAndGC = testObj.refCount;
>+    shouldBe('refAfterPass === refBeforePass + 50', 'true');
>+    shouldBe('refAfterPassAndGC < refAfterPass', 'true');
>+}
> </script>
> 
> <body onload="runtest()">
> 
>-Test that we can get an NPObject returned through a method on
>-an NPAPI Object.<P>
>-
>-Prints "SUCCESS" on success, "FAILURE" on failure.
>-
>-<embed name="plug" type="application/x-webkit-test-netscape">
>-
>-<div id=output>FAILURE</div>
>+    Test that we can get an NPObject returned through a method on
>+    an NPAPI Object.<P>
> 
>+    <embed name="plug" type="application/x-webkit-test-netscape">
> </body>
>-

Actions: View | Formatted Diff | Diff

Attachments on bug 186878: 343258 | 343266 | 343267 | 343274 | 343275 | 343277 | 343368 | 343385 | 343386 | 343388 | 343391 | 343412 | 343457 | 343461 | 343464 | 343477 | 343484 | 343489 | 343523 | 343546 | 343552 | 343645 | 343653