third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-styled-after-onload.html - Issue 2450793002: Call invalidatePaintForTickmarks() in Internals::addTextMatchMarker()

Side by Side Diff: third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-styled-after-onload.html

Issue 2450793002: Call invalidatePaintForTickmarks() in Internals::addTextMatchMarker() (Closed)

Patch Set: TestExpectations 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:

« no previous file with comments | « third_party/WebKit/LayoutTests/TestExpectations ('k') | third_party/WebKit/LayoutTests/fast/scrolling/scrollbar-tickmarks-styled-after-onload-expected.txt » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <head>	2 <head>

3 <script src="../../resources/ahem.js"></script>	3 <script src="../../resources/ahem.js"></script>

4 <style>	4 <style>

5 html {	5 html {

6 font: Ahem 10px;	6 font: Ahem 10px;

7 -webkit-font-smoothing: none;	7 -webkit-font-smoothing: none;

8 }	8 }

9 ::-webkit-scrollbar {	9 ::-webkit-scrollbar {

10 width: 13px;	10 width: 13px;

11 height: 13px;	11 height: 13px;

12 }	12 }

13 ::-webkit-scrollbar-track {	13 ::-webkit-scrollbar-track {

14 background-color:green;	14 background-color:green;

15 }	15 }

16 ::-webkit-scrollbar-thumb {	16 ::-webkit-scrollbar-thumb {

17 background-color:rgba(128, 128, 128, 0.8); /* Let tickmark shine through */	17 background-color:rgba(128, 128, 128, 0.8); /* Let tickmark shine through */

18 }	18 }

19 </style>	19 </style>

20 <script>	20 <script>

	21 if (window.testRunner) {

	22 testRunner.waitUntilDone();

	23 testRunner.dumpAsTextWithPixelResults();

	24 }

	25

21 function highlight()	26 function highlight()

22 {	27 {

23 var range = document.createRange();	28 var range = document.createRange();

24 var elt = document.getElementById('elt');	29 var elt = document.getElementById('elt');

25 range.selectNodeContents(elt);	30 range.selectNodeContents(elt);

	31

	32 // Enforce layout to test addTextMatchMarker() after the first layout.

	33 document.body.offsetTop;

	34

26 if (window.internals) {	35 if (window.internals) {

27 window.internals.addTextMatchMarker(range, true);	36 window.internals.addTextMatchMarker(range, true);

28 }	37 }

29 if (window.testRunner)	38 if (window.testRunner) {

30 testRunner.dumpAsTextWithPixelResults();	39 testRunner.notifyDone();

	40 }

31 }	41 }

32 </script>	42 </script>

33 </head>	43 </head>

34 <body onload="highlight();">	44 <body onload="setTimeout(highlight, 0)">
	Xianzhu 2016/10/31 15:59:07 Normally we use runAfterLayoutAndPaint(highlight, Normally we use runAfterLayoutAndPaint(highlight, true) (requiring <script src="../../resources/run-after-layout-and-paint.js"></script>) for this purpose. It ensures not only layout, but also paint invalidation and paint (full document cycle). I think this test needs the full document cycle, right? If you use it, you can omit the most of the added lines. hiroshige 2016/11/01 05:48:04 Thanks for suggestion! I use runAfterLayoutAndPain Show quoted text On 2016/10/31 15:59:07, Xianzhu wrote: > Normally we use runAfterLayoutAndPaint(highlight, true) (requiring <script > src="../../resources/run-after-layout-and-paint.js"></script>) for this purpose. > It ensures not only layout, but also paint invalidation and paint (full document > cycle). I think this test needs the full document cycle, right? If you use it, > you can omit the most of the added lines. Thanks for suggestion! I use runAfterLayoutAndPaint() in Patch Set 6, but the test crashing (assertion failure due to under-invalidation check), even without my change to Internals.cpp. Probably it is hitting another existing bug? Xianzhu 2016/11/01 06:41:22 The crash is expected without your patch because w Show quoted text On 2016/11/01 05:48:04, hiroshige wrote: > On 2016/10/31 15:59:07, Xianzhu wrote: > > Normally we use runAfterLayoutAndPaint(highlight, true) (requiring <script > > src="../../resources/run-after-layout-and-paint.js"></script>) for this > purpose. > > It ensures not only layout, but also paint invalidation and paint (full > document > > cycle). I think this test needs the full document cycle, right? If you use it, > > you can omit the most of the added lines. > > Thanks for suggestion! > I use runAfterLayoutAndPaint() in Patch Set 6, but the test crashing (assertion > failure due to under-invalidation check), even without my change to > Internals.cpp. Probably it is hitting another existing bug? The crash is expected without your patch because we don't invalidate paint in Internals::addTextMatchMarker(). Actually I have been assigned cluster-fuzz bugs about the crash, but I marked them WontFix because the crash was intentional: we called into internal code which intentionally didn't do paint invalidation. We expected the normal callers in production code to do it. Your patch should fix the crash by letting Internal::addTextMatchMarker() do paint invalidation. If there is still under-invalidation, then your patch seems incomplete to fix the issue. I can help you to investigate it. I scheduled dry run and will look at the results when it's available. BTW, does/will your patch fix scrollbar-tickmarks-styled.html?
35	45

36 <div style="height:600px"></div>	46 <div style="height:600px"></div>

37 <span id="elt" style="position:absolute; top:300px"> </span>	47 <span id="elt" style="position:absolute; top:300px"> </span>

38	48

39 </body>	49 </body>

40 </html>	50 </html>

OLD	NEW