Title: [93862] trunk/Source/WebCore
- Revision
- 93862
- Author
- [email protected]
- Date
- 2011-08-26 03:02:48 -0700 (Fri, 26 Aug 2011)
Log Message
2011-08-26 Nikolas Zimmermann <[email protected]>
[Qt] http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm fails intermittently
https://bugs.webkit.org/show_bug.cgi?id=65969
Reviewed by Zoltan Herczeg.
Fix intrinsic size negotiation flakiness, most visible on the Qt port.
The http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm testcase triggered
an assertion on a Qt debug build indicating that the HashSet updateLayoutAndStyleIfNeededRecursive()
operates on is mutated while its iterated, leading to an assertion/crash. Due the new forceLayoutParentViewIfNeeded()
logic it's no longer safe to directly use the children() HashSet in that method - we have to make a copy first.
The second part of the fix is to stop entering forceLayoutParentViewIfNeeded(), if the origin of that call
is forceLayoutParentViewIfNeeded() itself. Set m_inLayoutParentView to true before calling FrameView::layout()
on our parent frame view - this is only an optimization to avoid doing layout() twice.
The third part of the fix is to call updateWidgetPositions() on the parent FrameView, _before_ calling layout()
on the parent view itself, as the SVG document needs to report the correct intrinsic size (which can depend
on the host object/embed/iframe) when we're running RenderReplaced::layout() on the host renderer.
* page/FrameView.cpp:
(WebCore::FrameView::FrameView):
(WebCore::collectFrameViewChildren):
(WebCore::FrameView::forceLayoutParentViewIfNeeded):
(WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive):
* page/FrameView.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (93861 => 93862)
--- trunk/Source/WebCore/ChangeLog 2011-08-26 09:56:17 UTC (rev 93861)
+++ trunk/Source/WebCore/ChangeLog 2011-08-26 10:02:48 UTC (rev 93862)
@@ -1,3 +1,32 @@
+2011-08-26 Nikolas Zimmermann <[email protected]>
+
+ [Qt] http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm fails intermittently
+ https://bugs.webkit.org/show_bug.cgi?id=65969
+
+ Reviewed by Zoltan Herczeg.
+
+ Fix intrinsic size negotiation flakiness, most visible on the Qt port.
+
+ The http/tests/misc/object-embedding-svg-delayed-size-negotiation-2.htm testcase triggered
+ an assertion on a Qt debug build indicating that the HashSet updateLayoutAndStyleIfNeededRecursive()
+ operates on is mutated while its iterated, leading to an assertion/crash. Due the new forceLayoutParentViewIfNeeded()
+ logic it's no longer safe to directly use the children() HashSet in that method - we have to make a copy first.
+
+ The second part of the fix is to stop entering forceLayoutParentViewIfNeeded(), if the origin of that call
+ is forceLayoutParentViewIfNeeded() itself. Set m_inLayoutParentView to true before calling FrameView::layout()
+ on our parent frame view - this is only an optimization to avoid doing layout() twice.
+
+ The third part of the fix is to call updateWidgetPositions() on the parent FrameView, _before_ calling layout()
+ on the parent view itself, as the SVG document needs to report the correct intrinsic size (which can depend
+ on the host object/embed/iframe) when we're running RenderReplaced::layout() on the host renderer.
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::FrameView):
+ (WebCore::collectFrameViewChildren):
+ (WebCore::FrameView::forceLayoutParentViewIfNeeded):
+ (WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive):
+ * page/FrameView.h:
+
2011-08-26 Mikhail Naganov <[email protected]>
Web Inspector: [Chromium] Double clicking on numbers in Count & size columns doesn't toggle between values and percents in the Heap Snapshot.
Modified: trunk/Source/WebCore/page/FrameView.cpp (93861 => 93862)
--- trunk/Source/WebCore/page/FrameView.cpp 2011-08-26 09:56:17 UTC (rev 93861)
+++ trunk/Source/WebCore/page/FrameView.cpp 2011-08-26 10:02:48 UTC (rev 93862)
@@ -116,6 +116,9 @@
, m_fixedObjectCount(0)
, m_layoutTimer(this, &FrameView::layoutTimerFired)
, m_layoutRoot(0)
+#if ENABLE(SVG)
+ , m_inLayoutParentView(false)
+#endif
, m_hasPendingPostLayoutTasks(false)
, m_inSynchronousPostLayout(false)
, m_postLayoutTasksTimer(this, &FrameView::postLayoutTimerFired)
@@ -836,9 +839,25 @@
return onlyDuringLayout && layoutPending() ? 0 : m_layoutRoot;
}
+static inline void collectFrameViewChildren(FrameView* frameView, Vector<RefPtr<FrameView> >& frameViews)
+{
+ const HashSet<RefPtr<Widget> >* viewChildren = frameView->children();
+ ASSERT(viewChildren);
+
+ const HashSet<RefPtr<Widget> >::iterator end = viewChildren->end();
+ for (HashSet<RefPtr<Widget> >::iterator current = viewChildren->begin(); current != end; ++current) {
+ Widget* widget = (*current).get();
+ if (widget->isFrameView())
+ frameViews.append(static_cast<FrameView*>(widget));
+ }
+}
+
inline void FrameView::forceLayoutParentViewIfNeeded()
{
#if ENABLE(SVG)
+ if (m_inLayoutParentView)
+ return;
+
RenderPart* ownerRenderer = m_frame->ownerRenderer();
if (!ownerRenderer || !ownerRenderer->frame())
return;
@@ -851,6 +870,8 @@
if (!svgRoot->needsSizeNegotiationWithHostDocument())
return;
+ m_inLayoutParentView = true;
+
// Clear needs-size-negotiation flag in RenderSVGRoot, so the next call to our
// layout() method won't fire the size negotiation logic again.
svgRoot->scheduledSizeNegotiationWithHostDocument();
@@ -859,10 +880,18 @@
// Mark the owner renderer as needing layout.
ownerRenderer->setNeedsLayoutAndPrefWidthsRecalc();
- // Synchronously enter layout.
+ // Immediately relayout the child widgets, which can now calculate the SVG documents
+ // intrinsic size, negotiating with the parent object/embed/iframe.
+ RenderView* rootView = ownerRenderer->frame()->contentRenderer();
+ ASSERT(rootView);
+ rootView->updateWidgetPositions();
+
+ // Synchronously enter layout, to layout the view containing the host object/embed/iframe.
FrameView* frameView = ownerRenderer->frame()->view();
ASSERT(frameView);
frameView->layout();
+
+ m_inLayoutParentView = false;
#endif
}
@@ -2720,14 +2749,16 @@
if (needsLayout())
layout();
- const HashSet<RefPtr<Widget> >* viewChildren = children();
- HashSet<RefPtr<Widget> >::const_iterator end = viewChildren->end();
- for (HashSet<RefPtr<Widget> >::const_iterator current = viewChildren->begin(); current != end; ++current) {
- Widget* widget = (*current).get();
- if (widget->isFrameView())
- static_cast<FrameView*>(widget)->updateLayoutAndStyleIfNeededRecursive();
- }
+ // Grab a copy of the children() set, as it may be mutated by the following updateLayoutAndStyleIfNeededRecursive
+ // 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.
+ Vector<RefPtr<FrameView> > frameViews;
+ collectFrameViewChildren(this, frameViews);
+ const Vector<RefPtr<FrameView> >::iterator end = frameViews.end();
+ for (Vector<RefPtr<FrameView> >::iterator it = frameViews.begin(); it != end; ++it)
+ (*it)->updateLayoutAndStyleIfNeededRecursive();
+
// updateLayoutAndStyleIfNeededRecursive is called when we need to make sure style and layout are up-to-date before
// painting, so we need to flush out any deferred repaints too.
flushDeferredRepaints();
Modified: trunk/Source/WebCore/page/FrameView.h (93861 => 93862)
--- trunk/Source/WebCore/page/FrameView.h 2011-08-26 09:56:17 UTC (rev 93861)
+++ trunk/Source/WebCore/page/FrameView.h 2011-08-26 10:02:48 UTC (rev 93862)
@@ -400,6 +400,9 @@
bool m_layoutSchedulingEnabled;
bool m_inLayout;
+#if ENABLE(SVG)
+ bool m_inLayoutParentView;
+#endif
bool m_hasPendingPostLayoutTasks;
bool m_inSynchronousPostLayout;
int m_layoutCount;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
