Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(121)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp

Issue 2506353002: Incrementally build main thread scrolling reasons [spv2] (Closed)
Patch Set: Cleanup comments Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp ('K') | « third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp ('k') | third_party/WebKit/Source/core/paint/PaintPropertyTreePrinter.cpp » ('j') | third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
index dfc4dd7649571825a77990f8fc470fb8793b69ca..a3a0062f852569c30d297abfa02e0373992bc81b 100644
--- a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
@@ -2606,31 +2606,27 @@ TEST_P(PaintPropertyTreeBuilderTest,
"</div>"
"<div class='forceScroll'></div>");
Element* overflowA = document().getElementById("overflowA");
- EXPECT_FALSE(frameScroll()->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kThreadedScrollingDisabled));
+ EXPECT_FALSE(frameScroll()->threadedScrollingDisabled());
EXPECT_FALSE(overflowA->layoutObject()
->paintProperties()
->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kThreadedScrollingDisabled));
+ ->threadedScrollingDisabled());
document().settings()->setThreadedScrollingEnabled(false);
+ // TODO(pdr): The main thread scrolling setting should invalidate properties.
Xianzhu 2016/11/17 19:03:04 The TODO is fine. Just curious, should we make it
pdr. 2016/11/17 21:01:36 I want to remove it entirely but that was shot dow
+ document().view()->setNeedsPaintPropertyUpdate();
+ overflowA->layoutObject()->setNeedsPaintPropertyUpdate();
document().view()->updateAllLifecyclePhases();
- EXPECT_TRUE(frameScroll()->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kThreadedScrollingDisabled));
+ EXPECT_TRUE(frameScroll()->threadedScrollingDisabled());
EXPECT_TRUE(overflowA->layoutObject()
->paintProperties()
->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kThreadedScrollingDisabled));
+ ->threadedScrollingDisabled());
}
-// Disabled due to missing main thread scrolling property invalidation support.
-// See: https://crbug.com/664672
-TEST_P(
- PaintPropertyTreeBuilderTest,
- DISABLED_BackgroundAttachmentFixedMainThreadScrollReasonsWithNestedScrollers) {
+TEST_P(PaintPropertyTreeBuilderTest,
+ BackgroundAttachmentFixedMainThreadScrollReasonsWithNestedScrollers) {
setBodyInnerHTML(
"<style>"
" #overflowA {"
@@ -2663,63 +2659,45 @@ TEST_P(
Element* overflowA = document().getElementById("overflowA");
Element* overflowB = document().getElementById("overflowB");
- EXPECT_TRUE(frameScroll()->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_TRUE(
- overflowA->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_FALSE(
- overflowB->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
+ EXPECT_TRUE(frameScroll()->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_TRUE(overflowA->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_FALSE(overflowB->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
// Removing a main thread scrolling reason should update the entire tree.
overflowB->removeAttribute("class");
document().view()->updateAllLifecyclePhases();
- EXPECT_FALSE(frameScroll()->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_FALSE(
- overflowA->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_FALSE(
- overflowB->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
+ EXPECT_FALSE(frameScroll()->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_FALSE(overflowA->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_FALSE(overflowB->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
// Adding a main thread scrolling reason should update the entire tree.
overflowB->setAttribute(HTMLNames::classAttr, "backgroundAttachmentFixed");
document().view()->updateAllLifecyclePhases();
- EXPECT_TRUE(frameScroll()->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_TRUE(
- overflowA->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_FALSE(
- overflowB->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
+ EXPECT_TRUE(frameScroll()->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_TRUE(overflowA->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_FALSE(overflowB->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
}
-// Disabled due to missing main thread scrolling property invalidation support.
-// See: https://crbug.com/664672
-TEST_P(
- PaintPropertyTreeBuilderTest,
- DISABLED_BackgroundAttachmentFixedMainThreadScrollReasonsWithFixedScroller) {
+TEST_P(PaintPropertyTreeBuilderTest,
+ BackgroundAttachmentFixedMainThreadScrollReasonsWithFixedScroller) {
setBodyInnerHTML(
"<style>"
" #overflowA {"
@@ -2752,48 +2730,38 @@ TEST_P(
Element* overflowA = document().getElementById("overflowA");
Element* overflowB = document().getElementById("overflowB");
- EXPECT_FALSE(
- overflowA->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_FALSE(
- overflowB->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_TRUE(
- overflowB->layoutObject()
- ->paintProperties()
- ->scroll()
- ->parent()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
+ // This should be false. We are not as strict about main thread scrolling
+ // reasons as we could be.
+ EXPECT_TRUE(overflowA->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_FALSE(overflowB->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_TRUE(overflowB->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->parent()
+ ->isRoot());
// Removing a main thread scrolling reason should update the entire tree.
overflowB->removeAttribute("class");
document().view()->updateAllLifecyclePhases();
- EXPECT_FALSE(
- overflowA->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_FALSE(
- overflowB->layoutObject()
- ->paintProperties()
- ->scroll()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
- EXPECT_FALSE(
- overflowB->layoutObject()
- ->paintProperties()
- ->scroll()
- ->parent()
- ->hasMainThreadScrollingReasons(
- MainThreadScrollingReason::kHasBackgroundAttachmentFixedObjects));
+ EXPECT_FALSE(overflowA->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_FALSE(overflowB->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->hasBackgroundAttachmentFixedDescendants());
+ EXPECT_FALSE(overflowB->layoutObject()
+ ->paintProperties()
+ ->scroll()
+ ->parent()
+ ->hasBackgroundAttachmentFixedDescendants());
}
TEST_P(PaintPropertyTreeBuilderTest, PaintOffsetsUnderMultiColumn) {
« third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp ('K') | « third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp ('k') | third_party/WebKit/Source/core/paint/PaintPropertyTreePrinter.cpp » ('j') | third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698