Changeset 180062 in webkit

Timestamp:

Feb 13, 2015, 11:04:09 AM (10 years ago)

Author:

Simon Fraser

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):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/WebCore.exp.in (modified) (1 diff)
• WebCore/page/FrameTree.cpp (modified) (2 diffs)
• WebCore/page/FrameTree.h (modified) (3 diffs)
• WebCore/page/FrameView.cpp (modified) (1 diff)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/WebProcess/WebPage/WebPage.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-02-12  Simon Fraser  <simon.fraser@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  <ap@apple.com>
 

trunk/Source/WebCore/WebCore.exp.in

@@ -2146 +2146 @@
 __ZNK7WebCore9FrameTree12traverseNextEPKNS_5FrameE
 __ZNK7WebCore9FrameTree14isDescendantOfEPKNS_5FrameE
+__ZNK7WebCore9FrameTree20traverseNextRenderedEPKNS_5FrameE
 __ZNK7WebCore9FrameTree20traverseNextWithWrapEb
 __ZNK7WebCore9FrameTree24traversePreviousWithWrapEb

trunk/Source/WebCore/page/FrameTree.cpp

@@ -357 +357 @@
 }
 
+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
 {
@@ -429 +491 @@
     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());

trunk/Source/WebCore/page/FrameTree.h

@@ -57 +57 @@
 
         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;
@@ -88 +91 @@
         unsigned scopedChildCount(TreeScope*) const;
 
+        Frame* firstRenderedChild() const;
+        Frame* nextRenderedSibling() const;
+
         Frame& m_thisFrame;
 
@@ -105 +111 @@
 #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
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -3970 +3970 @@
     // 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());

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-02-12  Simon Fraser  <simon.fraser@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  <antti@apple.com>
 

trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp

@@ -4649 +4649 @@
     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;