Skip to content

Commit

Permalink
Unreviewed, reverting 278631@main.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=274038

Speculative revert was not correct. Performance regression was introduced by different change. Relanding

Reverted changeset:

"Unreviewed, reverting 278591@main."
https://bugs.webkit.org/show_bug.cgi?id=274023
https://commits.webkit.org/278631@main

Canonical link: https://commits.webkit.org/278657@main
  • Loading branch information
webkit-commit-queue authored and Constellation committed May 11, 2024
1 parent 1a3a465 commit a6189e1
Show file tree
Hide file tree
Showing 38 changed files with 554 additions and 101 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@
testRunner.notifyDone();
}, false);
</script>
<iframe width="300" height="300" src="http://localhost:8000/site-isolation/scrolling/resources/empty-iframe.html"></iframe>
<iframe width="300" height="300" src="http://localhost:8000/site-isolation/scrolling/resources/empty-iframe.html"></iframe>
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
Verifies scrolling tree for page with multiple iframes in the same process.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".



(scrolling tree
(frame scrolling node
(scrollable area size width=800 height=600)
(total content size width=800 height=600)
(last committed scroll position (0,0))
(scrollable area parameters
(horizontal scroll elasticity 2)
(vertical scroll elasticity 2)
(horizontal scrollbar mode 0)
(vertical scrollbar mode 0))
(layout viewport (0,0) width=800 height=600)
(min layoutViewport origin (0,0))
(max layoutViewport origin (0,0))
(behavior for fixed 1)
(frame hosting node
(has hosting context identifier )
(frame scrolling node
(scrollable area size width=300 height=400)
(total content size width=300 height=400)
(last committed scroll position (0,0))
(scrollable area parameters
(horizontal scroll elasticity 0)
(vertical scroll elasticity 0)
(horizontal scrollbar mode 0)
(vertical scrollbar mode 0))
(layout viewport (0,0) width=300 height=400)
(min layoutViewport origin (0,0))
(max layoutViewport origin (0,0))
(behavior for fixed 1)))
(frame hosting node
(has hosting context identifier )
(frame scrolling node
(scrollable area size width=200 height=100)
(total content size width=200 height=100)
(last committed scroll position (0,0))
(scrollable area parameters
(horizontal scroll elasticity 0)
(vertical scroll elasticity 0)
(horizontal scrollbar mode 0)
(vertical scrollbar mode 0))
(layout viewport (0,0) width=200 height=100)
(min layoutViewport origin (0,0))
(max layoutViewport origin (0,0))
(behavior for fixed 1)))))

Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<!-- webkit-test-runner [ SiteIsolationEnabled=true ] -->
<script src="/js-test-resources/js-test.js"></script>
<script src="/js-test-resources/ui-helper.js"></script>
<body>
<pre id="scrollingTree"></pre>
</body>
<script>
description("Verifies scrolling tree for page with multiple iframes in the same process.");
jsTestIsAsync = true;

if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

window.addEventListener('load', async () => {
if (!window.internals)
return

await UIHelper.ensurePresentationUpdate();
document.getElementById('scrollingTree').innerText = await UIHelper.getScrollingTree();
if (window.testRunner)
testRunner.notifyDone();
}, false);
</script>
<iframe width="300" height="400" src="http://localhost:8000/site-isolation/scrolling/resources/empty-iframe.html"></iframe>
<iframe width="200" height="100" src="http://localhost:8000/site-isolation/scrolling/resources/empty-iframe.html"></iframe>
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
Verifies that root frames are successfully removed from scrolling trees when removed from the DOM.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".



(scrolling tree
(frame scrolling node
(scrollable area size width=800 height=600)
(total content size width=800 height=600)
(last committed scroll position (0,0))
(scrollable area parameters
(horizontal scroll elasticity 2)
(vertical scroll elasticity 2)
(horizontal scrollbar mode 0)
(vertical scrollbar mode 0))
(layout viewport (0,0) width=800 height=600)
(min layoutViewport origin (0,0))
(max layoutViewport origin (0,0))
(behavior for fixed 1)
(frame hosting node
(has hosting context identifier )
(frame scrolling node
(scrollable area size width=300 height=400)
(total content size width=300 height=400)
(last committed scroll position (0,0))
(scrollable area parameters
(horizontal scroll elasticity 0)
(vertical scroll elasticity 0)
(horizontal scrollbar mode 0)
(vertical scrollbar mode 0))
(layout viewport (0,0) width=300 height=400)
(min layoutViewport origin (0,0))
(max layoutViewport origin (0,0))
(behavior for fixed 1)))))

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<!-- webkit-test-runner [ SiteIsolationEnabled=true ] -->
<script src="/js-test-resources/js-test.js"></script>
<script src="/js-test-resources/ui-helper.js"></script>
<body>
<pre id="scrollingTree"></pre>
</body>
<script>
description("Verifies that root frames are successfully removed from scrolling trees when removed from the DOM.");
jsTestIsAsync = true;

if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

window.addEventListener('load', async () => {
if (!window.internals)
return
frameToRemove.parentElement.removeChild(frameToRemove);
if (window.GCController)
GCController.collect();
document.getElementById('scrollingTree').innerText = await UIHelper.getScrollingTree();
setTimeout(function() {
if (window.testRunner)
testRunner.notifyDone();
}, 500);
}, false);
</script>
<iframe width="300" height="400" src="http://localhost:8000/site-isolation/scrolling/resources/empty-iframe.html"></iframe>
<iframe width="200" height="100" id=frameToRemove src="http://localhost:8000/site-isolation/scrolling/resources/empty-iframe.html"></iframe>
2 changes: 1 addition & 1 deletion LayoutTests/platform/ios/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -4646,7 +4646,7 @@ webkit.org/b/257904 http/tests/site-isolation/page-zoom.html [ Skip ]
webkit.org/b/257904 http/tests/site-isolation/selection-focus.html [ Skip ]
webkit.org/b/257904 http/tests/site-isolation/basic-iframe-render-output.html [ Failure Timeout Pass ]

webkit.org/b/269550 http/tests/site-isolation/scrolling/basic-scrolling-tree.html [ Skip ]
webkit.org/b/269550 http/tests/site-isolation/scrolling/ [ Skip ]

# This test can't be enabled on iOS until EventSenderProxyIOS::keyDown() is implemented.
http/tests/site-isolation/key-events.html [ Skip ]
Expand Down
4 changes: 4 additions & 0 deletions Source/WTF/WTF.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,7 @@
E4A0AD3D1A96253C00536DF6 /* WorkQueueCocoa.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E4A0AD3C1A96253C00536DF6 /* WorkQueueCocoa.cpp */; };
EB2C86D9267B275D0052CB9A /* CPUTimePOSIX.cpp in Sources */ = {isa = PBXBuildFile; fileRef = EB2C86D8267B275C0052CB9A /* CPUTimePOSIX.cpp */; };
EB61EDC72409CCC1001EFE36 /* SystemTracingCocoa.cpp in Sources */ = {isa = PBXBuildFile; fileRef = EB61EDC62409CCC0001EFE36 /* SystemTracingCocoa.cpp */; };
FA0C387B2BEAD56B00583842 /* SmallMap.h in Headers */ = {isa = PBXBuildFile; fileRef = FA0C387A2BEAD56B00583842 /* SmallMap.h */; settings = {ATTRIBUTES = (Private, ); }; };
FE032AD22463E43B0012D7C7 /* WTFConfig.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FE032AD02463E43B0012D7C7 /* WTFConfig.cpp */; };
FE03831C2ABC04F700A576A2 /* TZoneMallocInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = FE03831A2ABC04F700A576A2 /* TZoneMallocInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
FE03831D2ABC04F700A576A2 /* TZoneMalloc.h in Headers */ = {isa = PBXBuildFile; fileRef = FE03831B2ABC04F700A576A2 /* TZoneMalloc.h */; settings = {ATTRIBUTES = (Private, ); }; };
Expand Down Expand Up @@ -1818,6 +1819,7 @@
EF7D6CD59D8642A8A0DA86AD /* StackTrace.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = StackTrace.h; sourceTree = "<group>"; };
F6D67D3226F90142006E0349 /* Int128.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Int128.h; sourceTree = "<group>"; };
F72BBDB107FA424886178B9E /* SymbolImpl.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = SymbolImpl.cpp; sourceTree = "<group>"; };
FA0C387A2BEAD56B00583842 /* SmallMap.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SmallMap.h; sourceTree = "<group>"; };
FE032AD02463E43B0012D7C7 /* WTFConfig.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WTFConfig.cpp; sourceTree = "<group>"; };
FE032AD12463E43B0012D7C7 /* WTFConfig.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WTFConfig.h; sourceTree = "<group>"; };
FE03831A2ABC04F700A576A2 /* TZoneMallocInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TZoneMallocInlines.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2382,6 +2384,7 @@
A748744F17A0BDAE00FA04CB /* SixCharacterHash.cpp */,
A748745017A0BDAE00FA04CB /* SixCharacterHash.h */,
A8A4730C151A825B004123FF /* SizeLimits.cpp */,
FA0C387A2BEAD56B00583842 /* SmallMap.h */,
0FA6F39220CC73A200A03DCD /* SmallSet.cpp */,
7936D6A91C99F8AE000D1AED /* SmallSet.h */,
A30D412D1F0DE13F00B71954 /* SoftLinking.h */,
Expand Down Expand Up @@ -3427,6 +3430,7 @@
DD3DC89627A4BF8E007E5B61 /* SinglyLinkedList.h in Headers */,
DD3DC99427A4BF8E007E5B61 /* SinglyLinkedListWithTail.h in Headers */,
DD3DC92F27A4BF8E007E5B61 /* SixCharacterHash.h in Headers */,
FA0C387B2BEAD56B00583842 /* SmallMap.h in Headers */,
DD3DC8DE27A4BF8E007E5B61 /* SmallSet.h in Headers */,
DDF3076727C086CD006A526F /* SoftLinking.h in Headers */,
DD3DC8D727A4BF8E007E5B61 /* SoftLinking.h in Headers */,
Expand Down
1 change: 1 addition & 0 deletions Source/WTF/wtf/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ set(WTF_PUBLIC_HEADERS
SinglyLinkedList.h
SinglyLinkedListWithTail.h
SixCharacterHash.h
SmallMap.h
SmallSet.h
SoftLinking.h
SortedArrayMap.h
Expand Down
116 changes: 116 additions & 0 deletions Source/WTF/wtf/SmallMap.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/*
* Copyright (C) 2024 Apple 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:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
* EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, 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.
*/

#pragma once

#include <variant>
#include <wtf/HashMap.h>
#include <wtf/ScopedLambda.h>
#include <wtf/StdLibExtras.h>

namespace WTF {

// This is a map optimized for holding 0 or 1 items with no hashing or allocations in those cases.
template<typename Key, typename Value>
class SmallMap {
public:
using Pair = KeyValuePair<Key, Value>;
using Map = HashMap<Key, Value>;
using Storage = std::variant<std::monostate, Pair, Map>;

static_assert(sizeof(Pair) <= 4 * sizeof(uint64_t), "Don't use SmallMap with large types. It probably wastes memory.");

Value& ensure(const Key& key, const auto& functor)
{
ASSERT(Map::isValidKey(key));
if (std::holds_alternative<std::monostate>(m_storage)) {
m_storage = Pair { key, functor() };
return std::get<Pair>(m_storage).value;
}
if (auto* pair = std::get_if<Pair>(&m_storage)) {
if (pair->key == key)
return pair->value;
Map map;
map.add(WTFMove(pair->key), WTFMove(pair->value));
m_storage = WTFMove(map);
return std::get<Map>(m_storage).add(key, functor()).iterator->value;
}
return std::get<Map>(m_storage).ensure(key, functor).iterator->value;
}

void remove(const Key& key)
{
ASSERT(Map::isValidKey(key));
if (auto* pair = std::get_if<Pair>(&m_storage)) {
if (pair->key == key)
m_storage = std::monostate { };
} else if (auto* map = std::get_if<Map>(&m_storage))
map->remove(key);
}

const Value* get(const Key& key) const
{
ASSERT(Map::isValidKey(key));
if (auto* pair = std::get_if<Pair>(&m_storage)) {
if (pair->key == key)
return std::addressof(pair->value);
} else if (auto* map = std::get_if<Map>(&m_storage)) {
if (auto it = map->find(key); it != map->end())
return std::addressof(it->value);
}
return nullptr;
}

void forEach(const auto& callback) const
{
switchOn(m_storage, [&] (const std::monostate&) {
}, [&] (const Pair& pair) {
callback(pair.key, pair.value);
}, [&] (const Map& map) {
for (auto& [key, value] : map)
callback(key, value);
});
}

size_t size() const
{
return switchOn(m_storage, [&] (const std::monostate&) {
return 0u;
}, [&] (const Pair&) {
return 1u;
}, [&] (const Map& map) {
return map.size();
});
}

const Storage& rawStorage() const { return m_storage; }

private:
Storage m_storage;
};

} // namespace WTF

using WTF::SmallMap;
2 changes: 1 addition & 1 deletion Source/WebCore/dom/Document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6716,7 +6716,7 @@ void Document::setBackForwardCacheState(BackForwardCacheState state)
if (page && m_frame->isMainFrame()) {
frameView->resetScrollbarsAndClearContentsSize();
if (RefPtr scrollingCoordinator = page->scrollingCoordinator())
scrollingCoordinator->clearAllNodes();
scrollingCoordinator->clearAllNodes(m_frame->rootFrame().frameID());
}
}

Expand Down
5 changes: 4 additions & 1 deletion Source/WebCore/page/LocalFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,11 @@ LocalFrame::~LocalFrame()
localMainFrame->selfOnlyDeref();

if (isRootFrame()) {
if (RefPtr page = this->page())
if (RefPtr page = this->page()) {
page->removeRootFrame(*this);
if (auto* scrollingCoordinator = page->scrollingCoordinator())
scrollingCoordinator->rootFrameWasRemoved(frameID());
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/page/LocalFrameView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6197,6 +6197,11 @@ void LocalFrameView::scrollbarStyleDidChange()
scrollbarsController().updateScrollbarStyle();
}

FrameIdentifier LocalFrameView::rootFrameID() const
{
return m_frame->rootFrame().frameID();
}

} // namespace WebCore

#undef PAGE_ID
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/page/LocalFrameView.h
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,8 @@ class LocalFrameView final : public FrameView {

WEBCORE_EXPORT void scrollbarStyleDidChange();

FrameIdentifier rootFrameID() const final;

private:
explicit LocalFrameView(LocalFrame&);

Expand Down
Loading

0 comments on commit a6189e1

Please sign in to comment.