Bug 76699 – [WTR] Visited link tracking is not disabled properly

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Release%20WebKit2/r105504%20%2818894%29/results.html

fast/block/margin-collapse/056.html:
-        RenderInline {A} at (0,0) size 56x21 [color=#0000EE]
+        RenderInline {A} at (0,0) size 56x21 [color=#551A8B]

tables tests:
-          RenderInline {A} at (0,0) size 255x17 [color=#FFFF00]
+          RenderInline {A} at (0,0) size 255x17 [color=#999900]

[Csaba Osztrogonác]

And additionally it made 30 tests flakey ... :-/

[Csaba Osztrogonác]

Shouldn't we roll out r105461?

[Alexey Proskuryakov]

Changed results are quite likely to be improvements - did you find any actual regressions?

Are you sure that this change made tests flaky? It seems quite unlikely.

[Csaba Osztrogonác]

(In reply to comment #3)
> Changed results are quite likely to be improvements - did you find any actual regressions?
Without this patch WK1 and WK2 results for these tests were same,
but with this patch now the results are diverging. 
 
> Are you sure that this change made tests flaky? It seems quite unlikely.
I'm absolutely sure. Check the following two test:
r105460 - 1 failures 7 flakes 28 missing results - http://build.webkit.sed.hu/builders/x86-32%20Linux%20Qt%20Release%20WebKit2/builds/18859
r105461 - 9 failures 36 flakes 28 missing results - http://build.webkit.sed.hu/builders/x86-32%20Linux%20Qt%20Release%20WebKit2/builds/18860

[Alexey Proskuryakov]

> fast/block/margin-collapse/056.html

This test, and all tables/mozilla tests pass on Mac WebKit2.

r105461 was a fix for an obvious bug. While it could have caused changes in test results, I don't see how it could introduce failures or flakiness. Perhaps there is some Qt specific code that used to neuter this bug, and now misbehaves?

I'd like to help, but at this point, I think that some investigation from Qt side is needed.

[Rafael Brandao]

(In reply to comment #5)
> > fast/block/margin-collapse/056.html
> 
> This test, and all tables/mozilla tests pass on Mac WebKit2.
> 
> r105461 was a fix for an obvious bug. While it could have caused changes in test results, I don't see how it could introduce failures or flakiness. Perhaps there is some Qt specific code that used to neuter this bug, and now misbehaves?
> 
> I'd like to help, but at this point, I think that some investigation from Qt side is needed.

It seems very unlikely that those changes are behind this. I will take a look and try to figure out why this is happening, but when I ran just fast/block/margin-collapse/056.html I've got more diff than that pasted here (some widths were different).

[Rafael Brandao]

It's a bit hard to realize what could be wrong here, I'm getting the same diff for head at r105461 and the previous one: https://gist.github.com/3d7570e546cb05e5be68 (the same results I get from current head). Ossy, do you have any idea how a test that only uses colors red, aqua and yellow could have produced the color #551A8B (purple)? Maybe it is blending colors? It would be nice if there could be a way for us to setup an environment that would produce the same results as our buildbot or if we could access it to run tests remotely. I'm using x86-64, maybe this can only be reproduced on 32... Any ideas?

[Balazs Kelemen]

(In reply to comment #7)
> It's a bit hard to realize what could be wrong here, I'm getting the same diff for head at r105461 and the previous one: https://gist.github.com/3d7570e546cb05e5be68 (the same results I get from current head). Ossy, do you have any idea how a test that only uses colors red, aqua and yellow could have produced the color #551A8B (purple)? Maybe it is blending colors? It would be nice if there could be a way for us to setup an environment that would produce the same results as our buildbot or if we could access it to run tests remotely. I'm using x86-64, maybe this can only be reproduced on 32... Any ideas?

You could use QBAT (http://webkit.sed.hu/blog/20101028/qtwebkit-builder-and-tester-virtual-machine) or copy the amazon ec2 vm (azbest_hu can help you with that). If you use a distro other than Debian or Ubuntu that can also be a reason for mismatches, so you could set up an Ubuntu chroot that should produce the same results.

[Csaba Osztrogonác]

I skipped tables/mozilla/marvin/backgr_simple-table-cell.html because it fails always on Qt-WK2 because of this bug - http://trac.webkit.org/changeset/112232.

[Zan Dobersek]

These failures occur simply because of visited links being colored as visited - #551A8B in fast/block/margin-collapse/056.html and #999900 in tables tests.

Some tests then turn out as flaky as the links haven't been visited in the rerun due to a more narrow test list while others fail again because of specific tests being tested before again.

To stop this problem from occurring, the back-forward list should be cleared after each test. A similar fix was required for the Gtk WebKit1 port at bug #71052.

[Csaba Osztrogonác]

Szilárd started working on fix suggested in Comment #10.

[Andras Becsi]

Taking over as discussed with Ossy.

The issue seems a bit more complex than described in Comment #10.

Looks like the m_visitedLinkTable which is shared between the WebContext and WebProcess does not get cleaned properly / in time before style recalc, so links to previous tests appear in visited color.

I'll try to find a solution and upload a patch.

[Andras Becsi]

Created attachment 142012 [details]
proposed patch

[Andras Becsi]

(In reply to comment #13)
> Created an attachment (id=142012) [details]
> proposed patch

Looks like before r105461 local files were not tracked in visited links properly because of the missing file:// prefix. This concealed the issue that WebKitTestRunner always tracked visited links, thus tests which have local links to previous tests began to fail, producing links with visited color.

Not sure why this does not show up on Mac, though. WTR on Mac might not use PLATFORM_STRATEGIES.

[Jocelyn Turcotte]

Comment on attachment 142012 [details]
proposed patch

Looks good to me from what I've checked.
It would be nice to know how this end up being disabled on the Mac port, assuming that it doesn't use PageGroup::is/addVisitedLink either.

[Andras Becsi]

Comment on attachment 142012 [details]
proposed patch

Clearing flags on attachment: 142012

Committed r117248: <http://trac.webkit.org/changeset/117248>

[Andras Becsi]

All reviewed patches have been landed.  Closing bug.