Diff
Modified: trunk/LayoutTests/ChangeLog (221172 => 221173)
--- trunk/LayoutTests/ChangeLog 2017-08-25 00:28:56 UTC (rev 221172)
+++ trunk/LayoutTests/ChangeLog 2017-08-25 00:39:40 UTC (rev 221173)
@@ -1,3 +1,17 @@
+2017-08-24 Ryan Haddad <ryanhad...@apple.com>
+
+ Unreviewed, rolling out r221139.
+
+ This change did not resolve the LayoutTest assertion failure.
+
+ Reverted changeset:
+
+ "REGRESSION (r220052): ASSERTION FAILED:
+ !frame().isMainFrame() || !needsStyleRecalcOrLayout() in
+ WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive()"
+ https://bugs.webkit.org/show_bug.cgi?id=175270
+ http://trac.webkit.org/changeset/221139
+
2017-08-24 Youenn Fablet <you...@apple.com>
WPT harness errors on leaks and iOS-sim EWS bots
Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (221172 => 221173)
--- trunk/LayoutTests/platform/mac-wk2/TestExpectations 2017-08-25 00:28:56 UTC (rev 221172)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations 2017-08-25 00:39:40 UTC (rev 221173)
@@ -752,6 +752,8 @@
webkit.org/b/173946 [ Debug ] media/modern-media-controls/fullscreen-support/fullscreen-support-press.html [ Pass Failure ]
+webkit.org/b/175270 [ Debug ] plugins/crash-restoring-plugin-page-from-page-cache.html [ Skip ]
+
# CSS Regions are being phased out.
tiled-drawing/scrolling/non-fast-region/wheel-handler-in-region.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (221172 => 221173)
--- trunk/Source/WebCore/ChangeLog 2017-08-25 00:28:56 UTC (rev 221172)
+++ trunk/Source/WebCore/ChangeLog 2017-08-25 00:39:40 UTC (rev 221173)
@@ -1,3 +1,17 @@
+2017-08-24 Ryan Haddad <ryanhad...@apple.com>
+
+ Unreviewed, rolling out r221139.
+
+ This change did not resolve the LayoutTest assertion failure.
+
+ Reverted changeset:
+
+ "REGRESSION (r220052): ASSERTION FAILED:
+ !frame().isMainFrame() || !needsStyleRecalcOrLayout() in
+ WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive()"
+ https://bugs.webkit.org/show_bug.cgi?id=175270
+ http://trac.webkit.org/changeset/221139
+
2017-08-24 Jon Lee <jon...@apple.com>
Unreviewed.
Modified: trunk/Source/WebCore/dom/Document.cpp (221172 => 221173)
--- trunk/Source/WebCore/dom/Document.cpp 2017-08-25 00:28:56 UTC (rev 221172)
+++ trunk/Source/WebCore/dom/Document.cpp 2017-08-25 00:39:40 UTC (rev 221173)
@@ -460,7 +460,7 @@
, m_extensionStyleSheets(std::make_unique<ExtensionStyleSheets>(*this))
, m_visitedLinkState(std::make_unique<VisitedLinkState>(*this))
, m_markers(std::make_unique<DocumentMarkerController>(*this))
- , m_styleRecalcTimer([this] { updateStyleIfNeeded(); })
+ , m_styleRecalcTimer(*this, &Document::updateStyleIfNeeded)
, m_updateFocusAppearanceTimer(*this, &Document::updateFocusAppearanceTimerFired)
, m_documentCreationTime(MonotonicTime::now())
, m_scriptRunner(std::make_unique<ScriptRunner>(*this))
@@ -1887,21 +1887,20 @@
return false;
}
-bool Document::updateStyleIfNeeded()
+void Document::updateStyleIfNeeded()
{
ASSERT(isMainThread());
ASSERT(!view() || !view()->isPainting());
if (!view() || view()->isInRenderTreeLayout())
- return false;
+ return;
styleScope().flushPendingUpdate();
if (!needsStyleRecalc())
- return false;
+ return;
resolveStyle();
- return true;
}
void Document::updateLayout()
Modified: trunk/Source/WebCore/dom/Document.h (221172 => 221173)
--- trunk/Source/WebCore/dom/Document.h 2017-08-25 00:28:56 UTC (rev 221172)
+++ trunk/Source/WebCore/dom/Document.h 2017-08-25 00:39:40 UTC (rev 221173)
@@ -544,7 +544,7 @@
enum class ResolveStyleType { Normal, Rebuild };
void resolveStyle(ResolveStyleType = ResolveStyleType::Normal);
- WEBCORE_EXPORT bool updateStyleIfNeeded();
+ WEBCORE_EXPORT void updateStyleIfNeeded();
bool needsStyleRecalc() const;
unsigned lastStyleUpdateSizeForTesting() const { return m_lastStyleUpdateSizeForTesting; }
Modified: trunk/Source/WebCore/page/FrameView.cpp (221172 => 221173)
--- trunk/Source/WebCore/page/FrameView.cpp 2017-08-25 00:28:56 UTC (rev 221172)
+++ trunk/Source/WebCore/page/FrameView.cpp 2017-08-25 00:39:40 UTC (rev 221173)
@@ -4575,16 +4575,6 @@
ScrollView::paintOverhangAreas(context, horizontalOverhangArea, verticalOverhangArea, dirtyRect);
}
-static void appendRenderedChildren(FrameView& view, Deque<Ref<FrameView>, 16>& deque)
-{
- for (Frame* frame = view.frame().tree().firstRenderedChild(); frame; frame = frame->tree().nextRenderedSibling()) {
- if (frame->view())
- deque.append(*frame->view());
- }
-}
-
-// FIXME: Change the one remaining caller of this to use appendRenderedChildren above instead,
-// and then remove FrameViewList and renderedChildFrameViews.
FrameView::FrameViewList FrameView::renderedChildFrameViews() const
{
FrameViewList childViews;
@@ -4598,50 +4588,32 @@
void FrameView::updateLayoutAndStyleIfNeededRecursive()
{
- // The number "4" here is empirically determined as the number of passes needed to
- // make sure we don't need more style recalculation or layout; we have not seen a
- // case where we need more than 3 passes, so 4 is a bit more than needed in any
- // normal case. But it's not clear that anything prevents an indefinite number of
- // passes from being needed. So it would be better to instead have a firm guarantee
- // of the number of times this needs to be done, or come up with a way to do this
- // in one pass without a loop.
- const unsigned maxUpdatePasses = 4;
+ // We have to crawl our entire tree looking for any FrameViews that need
+ // layout and make sure they are up to date.
+ // Mac actually tests for intersection with the dirty region and tries not to
+ // update layout for frames that are outside the dirty region. Not only does this seem
+ // pointless (since those frames will have set a zero timer to layout anyway), but
+ // it is also incorrect, since if two frames overlap, the first could be excluded from the dirty
+ // region but then become included later by the second frame adding rects to the dirty region
+ // when it lays out.
AnimationUpdateBlock animationUpdateBlock(&frame().animation());
- auto updateOnce = [this] {
- auto updateOneFrame = [] (FrameView& view) {
- bool didWork = view.frame().document()->updateStyleIfNeeded();
- if (view.needsLayout()) {
- view.layout();
- didWork = true;
- }
- return didWork;
- };
+ frame().document()->updateStyleIfNeeded();
- bool didWork = false;
+ if (needsLayout())
+ layout();
- // Use a copy of the child frame list because it can change while updating.
- Deque<Ref<FrameView>, 16> views;
- views.append(*this);
- while (!views.isEmpty()) {
- auto view = views.takeFirst();
- if (updateOneFrame(view.get()))
- didWork = true;
- appendRenderedChildren(view.get(), views);
- }
+ // Grab a copy of the child views, as the list may be mutated by the following updateLayoutAndStyleIfNeededRecursive
+ // calls, as they can potentially re-enter a layout of the parent frame view.
+ for (auto& frameView : renderedChildFrameViews())
+ frameView->updateLayoutAndStyleIfNeededRecursive();
- return didWork;
- };
+ // A child frame may have dirtied us during its layout.
+ frame().document()->updateStyleIfNeeded();
+ if (needsLayout())
+ layout();
- for (unsigned i = 0; i < maxUpdatePasses; ++i) {
- if (!updateOnce())
- break;
- }
-
- // FIXME: Unclear why it's appropriate to skip this assertion for non-main frames.
- // The need for this may be obsolete and a leftover from when this fucntion was
- // implemented by recursively calling itself.
ASSERT(!frame().isMainFrame() || !needsStyleRecalcOrLayout());
}
