- Revision
- 180062
- Author
- simon.fra...@apple.com
- Date
- 2015-02-13 11:04:09 -0800 (Fri, 13 Feb 2015)
Log Message
determinePrimarySnapshottedPlugIn() should only traverse visible Frames
https://bugs.webkit.org/show_bug.cgi?id=141547
Part of rdar://problem/18445733.
Reviewed by Anders Carlsson.
Source/WebCore:
There's an expectation from clients that FrameView::updateLayoutAndStyleIfNeededRecursive()
updates layout in all frames, but it uses the widget tree, so only hits frames
that are parented via renderers (i.e. not display:none frames or their descendants).
Moving towards a future where we remove Widgets, fix by adding a FrameTree
traversal function that only finds rendered frames (those with an ownerRenderer).
Not testable.
* page/FrameTree.cpp:
(WebCore::FrameTree::firstRenderedChild):
(WebCore::FrameTree::nextRenderedSibling):
(WebCore::FrameTree::traverseNextRendered):
(printFrames):
* page/FrameTree.h:
* page/FrameView.cpp:
(WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive):
Source/WebKit2:
Use FrameTree::traverseNextRendered() to avoid doing things in unrendered frames
which are not guaranteed to have been laid out.
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::determinePrimarySnapshottedPlugIn):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (180061 => 180062)
--- trunk/Source/WebCore/ChangeLog 2015-02-13 19:01:54 UTC (rev 180061)
+++ trunk/Source/WebCore/ChangeLog 2015-02-13 19:04:09 UTC (rev 180062)
@@ -1,3 +1,29 @@
+2015-02-12 Simon Fraser <simon.fra...@apple.com>
+
+ determinePrimarySnapshottedPlugIn() should only traverse visible Frames
+ https://bugs.webkit.org/show_bug.cgi?id=141547
+ Part of rdar://problem/18445733.
+
+ Reviewed by Anders Carlsson.
+
+ There's an expectation from clients that FrameView::updateLayoutAndStyleIfNeededRecursive()
+ updates layout in all frames, but it uses the widget tree, so only hits frames
+ that are parented via renderers (i.e. not display:none frames or their descendants).
+
+ Moving towards a future where we remove Widgets, fix by adding a FrameTree
+ traversal function that only finds rendered frames (those with an ownerRenderer).
+
+ Not testable.
+
+ * page/FrameTree.cpp:
+ (WebCore::FrameTree::firstRenderedChild):
+ (WebCore::FrameTree::nextRenderedSibling):
+ (WebCore::FrameTree::traverseNextRendered):
+ (printFrames):
+ * page/FrameTree.h:
+ * page/FrameView.cpp:
+ (WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive):
+
2015-02-13 Alexey Proskuryakov <a...@apple.com>
TimerBase::m_heapInsertionOrder calculation is racy
Modified: trunk/Source/WebCore/WebCore.exp.in (180061 => 180062)
--- trunk/Source/WebCore/WebCore.exp.in 2015-02-13 19:01:54 UTC (rev 180061)
+++ trunk/Source/WebCore/WebCore.exp.in 2015-02-13 19:04:09 UTC (rev 180062)
@@ -2145,6 +2145,7 @@
__ZNK7WebCore9FrameTree10childCountEv
__ZNK7WebCore9FrameTree12traverseNextEPKNS_5FrameE
__ZNK7WebCore9FrameTree14isDescendantOfEPKNS_5FrameE
+__ZNK7WebCore9FrameTree20traverseNextRenderedEPKNS_5FrameE
__ZNK7WebCore9FrameTree20traverseNextWithWrapEb
__ZNK7WebCore9FrameTree24traversePreviousWithWrapEb
__ZNK7WebCore9FrameTree3topEv
Modified: trunk/Source/WebCore/page/FrameTree.cpp (180061 => 180062)
--- trunk/Source/WebCore/page/FrameTree.cpp 2015-02-13 19:01:54 UTC (rev 180061)
+++ trunk/Source/WebCore/page/FrameTree.cpp 2015-02-13 19:04:09 UTC (rev 180062)
@@ -356,6 +356,68 @@
return nullptr;
}
+Frame* FrameTree::firstRenderedChild() const
+{
+ Frame* child = firstChild();
+ if (!child)
+ return nullptr;
+
+ if (child->ownerRenderer())
+ return child;
+
+ while ((child = child->tree().nextSibling())) {
+ if (child->ownerRenderer())
+ return child;
+ }
+
+ return nullptr;
+}
+
+Frame* FrameTree::nextRenderedSibling() const
+{
+ Frame* sibling = &m_thisFrame;
+
+ while ((sibling = sibling->tree().nextSibling())) {
+ if (sibling->ownerRenderer())
+ return sibling;
+ }
+
+ return nullptr;
+}
+
+Frame* FrameTree::traverseNextRendered(const Frame* stayWithin) const
+{
+ Frame* child = firstRenderedChild();
+ if (child) {
+ ASSERT(!stayWithin || child->tree().isDescendantOf(stayWithin));
+ return child;
+ }
+
+ if (&m_thisFrame == stayWithin)
+ return nullptr;
+
+ Frame* sibling = nextRenderedSibling();
+ if (sibling) {
+ ASSERT(!stayWithin || sibling->tree().isDescendantOf(stayWithin));
+ return sibling;
+ }
+
+ Frame* frame = &m_thisFrame;
+ while (!sibling && (!stayWithin || frame->tree().parent() != stayWithin)) {
+ frame = frame->tree().parent();
+ if (!frame)
+ return nullptr;
+ sibling = frame->tree().nextRenderedSibling();
+ }
+
+ if (frame) {
+ ASSERT(!stayWithin || !sibling || sibling->tree().isDescendantOf(stayWithin));
+ return sibling;
+ }
+
+ return nullptr;
+}
+
Frame* FrameTree::traverseNextWithWrap(bool wrap) const
{
if (Frame* result = traverseNext())
@@ -428,8 +490,10 @@
printIndent(indent);
printf(" renderView=%p\n", view ? view->renderView() : nullptr);
printIndent(indent);
- printf(" document=%p (needs style recalc %d)\n", frame.document(), frame.document() ? frame.document()->needsStyleRecalc() : false);
+ printf(" ownerRenderer=%p\n", frame.ownerRenderer());
printIndent(indent);
+ printf(" document=%p (needs style recalc %d)\n", frame.document(), frame.document() ? frame.document()->childNeedsStyleRecalc() : false);
+ printIndent(indent);
printf(" uri=%s\n", frame.document()->documentURI().utf8().data());
for (WebCore::Frame* child = frame.tree().firstChild(); child; child = child->tree().nextSibling())
Modified: trunk/Source/WebCore/page/FrameTree.h (180061 => 180062)
--- trunk/Source/WebCore/page/FrameTree.h 2015-02-13 19:01:54 UTC (rev 180061)
+++ trunk/Source/WebCore/page/FrameTree.h 2015-02-13 19:04:09 UTC (rev 180062)
@@ -56,7 +56,10 @@
Frame* lastChild() const { return m_lastChild; }
WEBCORE_EXPORT bool isDescendantOf(const Frame* ancestor) const;
- WEBCORE_EXPORT Frame* traverseNext(const Frame* stayWithin = 0) const;
+
+ WEBCORE_EXPORT Frame* traverseNext(const Frame* stayWithin = nullptr) const;
+ // Rendered means being the main frame or having an ownerRenderer. It may not have been parented in the Widget tree yet (see WidgetHierarchyUpdatesSuspensionScope).
+ WEBCORE_EXPORT Frame* traverseNextRendered(const Frame* stayWithin = nullptr) const;
WEBCORE_EXPORT Frame* traverseNextWithWrap(bool) const;
WEBCORE_EXPORT Frame* traversePreviousWithWrap(bool) const;
@@ -87,6 +90,9 @@
Frame* scopedChild(const AtomicString& name, TreeScope*) const;
unsigned scopedChildCount(TreeScope*) const;
+ Frame* firstRenderedChild() const;
+ Frame* nextRenderedSibling() const;
+
Frame& m_thisFrame;
Frame* m_parent;
@@ -104,7 +110,7 @@
#ifndef NDEBUG
// Outside the WebCore namespace for ease of invocation from gdb.
-void showFrameTree(const WebCore::Frame*);
+WEBCORE_EXPORT void showFrameTree(const WebCore::Frame*);
#endif
#endif // FrameTree_h
Modified: trunk/Source/WebCore/page/FrameView.cpp (180061 => 180062)
--- trunk/Source/WebCore/page/FrameView.cpp 2015-02-13 19:01:54 UTC (rev 180061)
+++ trunk/Source/WebCore/page/FrameView.cpp 2015-02-13 19:04:09 UTC (rev 180062)
@@ -3969,6 +3969,7 @@
// calls, as they can potentially re-enter a layout of the parent frame view, which may add/remove scrollbars
// and thus mutates the children() set.
// We use the Widget children because walking the Frame tree would include display:none frames.
+ // FIXME: use FrameTree::traverseNextRendered().
Vector<Ref<FrameView>, 16> childViews;
childViews.reserveInitialCapacity(children().size());
for (auto& widget : children()) {
Modified: trunk/Source/WebKit2/ChangeLog (180061 => 180062)
--- trunk/Source/WebKit2/ChangeLog 2015-02-13 19:01:54 UTC (rev 180061)
+++ trunk/Source/WebKit2/ChangeLog 2015-02-13 19:04:09 UTC (rev 180062)
@@ -1,3 +1,17 @@
+2015-02-12 Simon Fraser <simon.fra...@apple.com>
+
+ determinePrimarySnapshottedPlugIn() should only traverse visible Frames
+ https://bugs.webkit.org/show_bug.cgi?id=141547
+ Part of rdar://problem/18445733.
+
+ Reviewed by Anders Carlsson.
+
+ Use FrameTree::traverseNextRendered() to avoid doing things in unrendered frames
+ which are not guaranteed to have been laid out.
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::determinePrimarySnapshottedPlugIn):
+
2015-02-13 Antti Koivisto <an...@apple.com>
WorkQueue should support concurrent queues
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp (180061 => 180062)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2015-02-13 19:01:54 UTC (rev 180061)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp 2015-02-13 19:04:09 UTC (rev 180062)
@@ -4648,7 +4648,7 @@
HTMLPlugInImageElement* candidatePlugIn = nullptr;
unsigned candidatePlugInArea = 0;
- for (Frame* frame = &mainFrame; frame; frame = frame->tree().traverseNext()) {
+ for (Frame* frame = &mainFrame; frame; frame = frame->tree().traverseNextRendered()) {
if (!frame->loader().subframeLoader().containsPlugins())
continue;
if (!frame->document() || !frame->view())
