Title: [221596] trunk/Source/WebCore
- Revision
- 221596
- Author
- [email protected]
- Date
- 2017-09-04 16:50:04 -0700 (Mon, 04 Sep 2017)
Log Message
Follow up FrameView::updateLayoutAndStyleIfNeededRecursive changes with related improvements
https://bugs.webkit.org/show_bug.cgi?id=176277
Reviewed by Antti Koivisto.
* page/FrameView.cpp:
(WebCore::FrameView::needsStyleRecalcOrLayout): Deleted. This function was only used
by an assertion inside updateLayoutAndStyleIfNeededRecursive, and thus there is no reason
for it to be in the header file, or for it to be a public member function.
(WebCore::appendRenderedChildren): Deleted. This function was only used inside
updateLayoutAndStyleIfNeededRecursive, and it is now packaged in an even better way
for efficient use inside that function.
(WebCore::FrameView::renderedChildFrameViews): Deleted. This function was only used
inside needsStyleRecalcOrLayout, and it's now packaged in a better way inside that function.
(WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive): Created a new lambda called
nextRendereredDescendant that packages up the process of repeatedly iterating this view
and all of its descendants in an easy-to-use way. Replaces both of the functions above.
Rewrote to use it; it made the logic clear enough that it was good to get rid of the
updateOneFrame lambda, too. Added two separate functions, one that checks for needed
style recalculation and a separate one that checked for needed layout. Using those,
replaced the old single assertion with two separate assertions.
* page/FrameView.h: Removed needsStyleRecalcOrLayout, renderedChildFrameViews, and
FrameViewList.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (221595 => 221596)
--- trunk/Source/WebCore/ChangeLog 2017-09-04 23:36:20 UTC (rev 221595)
+++ trunk/Source/WebCore/ChangeLog 2017-09-04 23:50:04 UTC (rev 221596)
@@ -1,3 +1,30 @@
+2017-09-03 Darin Adler <[email protected]>
+
+ Follow up FrameView::updateLayoutAndStyleIfNeededRecursive changes with related improvements
+ https://bugs.webkit.org/show_bug.cgi?id=176277
+
+ Reviewed by Antti Koivisto.
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::needsStyleRecalcOrLayout): Deleted. This function was only used
+ by an assertion inside updateLayoutAndStyleIfNeededRecursive, and thus there is no reason
+ for it to be in the header file, or for it to be a public member function.
+ (WebCore::appendRenderedChildren): Deleted. This function was only used inside
+ updateLayoutAndStyleIfNeededRecursive, and it is now packaged in an even better way
+ for efficient use inside that function.
+ (WebCore::FrameView::renderedChildFrameViews): Deleted. This function was only used
+ inside needsStyleRecalcOrLayout, and it's now packaged in a better way inside that function.
+ (WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive): Created a new lambda called
+ nextRendereredDescendant that packages up the process of repeatedly iterating this view
+ and all of its descendants in an easy-to-use way. Replaces both of the functions above.
+ Rewrote to use it; it made the logic clear enough that it was good to get rid of the
+ updateOneFrame lambda, too. Added two separate functions, one that checks for needed
+ style recalculation and a separate one that checked for needed layout. Using those,
+ replaced the old single assertion with two separate assertions.
+
+ * page/FrameView.h: Removed needsStyleRecalcOrLayout, renderedChildFrameViews, and
+ FrameViewList.
+
2017-09-04 Wenson Hsieh <[email protected]>
[iOS DnD] Refactor drag and drop logic in WKContentView in preparation for supporting multiple drag items in a drag session
Modified: trunk/Source/WebCore/page/FrameView.cpp (221595 => 221596)
--- trunk/Source/WebCore/page/FrameView.cpp 2017-09-04 23:36:20 UTC (rev 221595)
+++ trunk/Source/WebCore/page/FrameView.cpp 2017-09-04 23:50:04 UTC (rev 221596)
@@ -3151,25 +3151,6 @@
return m_layoutTimer.isActive();
}
-bool FrameView::needsStyleRecalcOrLayout(bool includeSubframes) const
-{
- if (frame().document() && frame().document()->childNeedsStyleRecalc())
- return true;
-
- if (needsLayout())
- return true;
-
- if (!includeSubframes)
- return false;
-
- for (auto& frameView : renderedChildFrameViews()) {
- if (frameView->needsStyleRecalcOrLayout())
- return true;
- }
-
- return false;
-}
-
bool FrameView::needsLayout() const
{
// This can return true in cases where the document does not have a body yet.
@@ -4575,30 +4556,9 @@
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;
- for (Frame* frame = m_frame->tree().firstRenderedChild(); frame; frame = frame->tree().nextRenderedSibling()) {
- if (frame->view())
- childViews.append(*frame->view());
- }
-
- return childViews;
-}
-
void FrameView::updateLayoutAndStyleIfNeededRecursive()
{
- // Style updating, render tree, creation, and layout needs to be done multiple times
+ // Style updating, render tree creation, and layout needs to be done multiple times
// for more than one reason. But one reason is that when an <object> element determines
// what it needs to load a subframe, a second pass is needed. That requires update
// passes equal to the number of levels of DOM nesting. That is why this number is large.
@@ -4611,40 +4571,62 @@
AnimationUpdateBlock animationUpdateBlock(&frame().animation());
- auto updateOnce = [this] {
- auto updateOneFrame = [] (FrameView& view) {
- bool didWork = view.frame().document()->updateStyleIfNeeded();
- if (view.needsLayout()) {
- view.layout();
- didWork = true;
+ using DescendantsDeque = Deque<Ref<FrameView>, 16>;
+ auto nextRenderedDescendant = [this] (DescendantsDeque& descendantsDeque) -> RefPtr<FrameView> {
+ if (descendantsDeque.isEmpty())
+ descendantsDeque.append(*this);
+ else {
+ // Append renderered children after processing the parent, in case the processing
+ // affects the set of rendered children.
+ auto previousView = descendantsDeque.takeFirst();
+ for (auto* frame = previousView->frame().tree().firstRenderedChild(); frame; frame = frame->tree().nextRenderedSibling()) {
+ if (auto* view = frame->view())
+ descendantsDeque.append(*view);
}
- return didWork;
- };
+ if (descendantsDeque.isEmpty())
+ return nullptr;
+ }
+ return descendantsDeque.first().ptr();
+ };
+ for (unsigned i = 0; i < maxUpdatePasses; ++i) {
bool didWork = false;
-
- // 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()))
+ DescendantsDeque deque;
+ while (auto view = nextRenderedDescendant(deque)) {
+ if (view->frame().document()->updateStyleIfNeeded())
didWork = true;
- appendRenderedChildren(view.get(), views);
+ if (view->needsLayout()) {
+ view->layout();
+ didWork = true;
+ }
}
+ if (!didWork)
+ break;
+ }
- return didWork;
+#if !ASSERT_DISABLED
+ auto needsStyleRecalc = [&] {
+ DescendantsDeque deque;
+ while (auto view = nextRenderedDescendant(deque)) {
+ auto* document = view->frame().document();
+ if (document && document->childNeedsStyleRecalc())
+ return true;
+ }
+ return false;
};
- for (unsigned i = 0; i < maxUpdatePasses; ++i) {
- if (!updateOnce())
- break;
- }
+ auto needsLayout = [&] {
+ DescendantsDeque deque;
+ while (auto view = nextRenderedDescendant(deque)) {
+ if (view->needsLayout())
+ return true;
+ }
+ return false;
+ };
+#endif
- // 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());
+ ASSERT(!needsStyleRecalc());
+ ASSERT(!needsLayout());
}
bool FrameView::qualifiesAsVisuallyNonEmpty() const
Modified: trunk/Source/WebCore/page/FrameView.h (221595 => 221596)
--- trunk/Source/WebCore/page/FrameView.h 2017-09-04 23:36:20 UTC (rev 221595)
+++ trunk/Source/WebCore/page/FrameView.h 2017-09-04 23:50:04 UTC (rev 221596)
@@ -122,8 +122,6 @@
WEBCORE_EXPORT void setNeedsLayout();
void setViewportConstrainedObjectsNeedLayout();
- bool needsStyleRecalcOrLayout(bool includeSubframes = true) const;
-
bool needsFullRepaint() const { return m_needsFullRepaint; }
WEBCORE_EXPORT bool renderedCharactersExceed(unsigned threshold);
@@ -606,9 +604,6 @@
const HashSet<Widget*>& widgetsInRenderTree() const { return m_widgetsInRenderTree; }
- typedef Vector<Ref<FrameView>, 16> FrameViewList;
- FrameViewList renderedChildFrameViews() const;
-
void addTrackedRepaintRect(const FloatRect&);
// exposedRect represents WebKit's understanding of what part
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
