1557371 - Load all XUL reftests with system privilege

Status: RESOLVED FIXED

(Testing :: Reftest, task)

Description

[Brendan Dahl [:bdahl]]

This will have two benefits:

1. Align test setup with shipping Firefox - We don't allow content privilege XUL in shipping versions of Firefox, so having the tests be chrome would be more realistic to our use case.

2. Support the XUL to XHTML migration. These files will soon become XHTML files, but will still need to load XUL elements, so they'll need to be marked as chrome privileged to continue working.

Comment 1

[Brendan Dahl [:bdahl]]

Attachment: Bug 1557371 - Part 1 - Load all XUL reftests with chrome privilege. r=dbaron

This will have two benefits:

1. Align test setup with shipping Firefox - We don't allow content
   privilege XUL in shipping versions of Firefox, so having the tests be
   chrome would be more realistic to our use case.

2. Support the XUL to XHTML migration. These files will soon become XHTML
   files, but will still need to load XUL elements, so they'll need to be
   marked as chrome privileged to continue working.

One test (404149-1.xul) is now disabled, since it fails when loaded as
chrome. Bug 1557383 was file to address this.

Comment 2

[Brendan Dahl [:bdahl]]

Attachment: Bug 1557371 - Part 2 - Load all XUL crashtests with chrome privilege. r=dbaron

See previous commit.

Depends on D33986

Comment 3

[Brendan Dahl [:bdahl]]

One downside of this approach is ./mach test is no longer able to find a reftest file by filename alone.

Comment 5

[Brendan Dahl [:bdahl]]

For ensuring the reftests are still doing what we expect I did a few things:

1. For == tests, load one with a "chrome" url and one without. The results aren't looking so pretty.
2. For != tests, split them into two == tests where we compare the chrome and non-chrome version of each file. Results there aren't as bad, but also not green.

As for crashtests, I think we'll need to do the file opening and error logging that dbaron suggested in phabricator.

Comment 6

[Brendan Dahl [:bdahl]]

On further inspection it's looking like a lot of tests were actually broken when using file url's to load. I need to go through them all, but so far it appears most are actually fixed by loading them with chrome.

Comment 7

[Brendan Dahl [:bdahl]]

I went through all the reftest failures with the changes mentioned in comment #5. Most tests improved when loaded as chrome! It looks like text for buttons and labels was not working correctly when xul was loaded with content privilege. Also, it looks like radio boxes and checkboxes were broken in content (the reftests were blank), but now they're fixed under chrome. There were a few I was unsure about, but after going through them I think they're all improvements as well, but if someone wants to spot check a few that would be helpful.

Results:

== Reftests:

Linux:
  checkbox-dynamic-change-ref.xul - improved, checkbox is now visible
  radio-dynamic-change-ref.xul - improved, radio is now visible
  menulist-shrinkwrap-1-ref.xul - unsure, drop down is larger
  menulist-shrinkwrap-2-ref.xul - unsure, text is now visible but button is cut off
  accesskey-ref.xul - unsure, access key indicator (X) is now visible
  tree-row-outline-1-ref.xul - improved, icon now visible in header
  treechildren-padding-percent-1-ref.xul - improved, icon now visible in header
  treetwisty-svg-context-paint-1-ref.xul - improved, icon now visible in header
  470711-1-ref.xul - improved, tab outline now shows
  676387-1-ref.xul - improved, button text now shows
  261826-1-ref.xul - improved, tab outline now shows
  272646-1-ref.xul - unsure, more text now there
  272646-2-ref.xul - improved, tab outline now shows
  309914-1-ref.xul - improved, button text now visible
  374719-1-ref.xul - unsure, looks like grid header now visible
  468473-1-ref.xul - improved, green box now shows
MacOS:
  470711-1-ref.xul - improved, tabs now appear
  searchfield-mirrored-when-rtl-ref.xul - unsure, search box now larger (looks better)
  676387-1-ref.xul - improved, button text now shows
  482681-ref.xul - unsure, heights of buttons changes slightly
  radiosize-ref.xul - improved, radio buttons now visible
  checkboxsize-ref.xul - improved, checkboxes now visible
  nostretch-ref.xul - improved, button text now visible
  261826-1-ref.xul - improved, tabs now appear
  272646-1-ref.xul - unsure, more text now there
  272646-2-ref.xul - improved, tabs now appear
  309914-1-ref.xul - improved, button text now visible
  374719-1-ref.xul - unsure, looks like grid header now visible (improvement?)
  468473-1-ref.xul - improved, green box now shows
  checkbox-dynamic-change-ref.xul - improved, checkbox is now visible
  radio-dynamic-change-ref.xul - improved, radio is now visible
Windows 10:
  261826-1-ref.xul - improved, tabs now appear
  272646-1-ref.xul - unsure, more text now there
  272646-2-ref.xul - improved, tabs now appear
  309914-1-ref.xul - improved, button text now visible
  468473-1-ref.xul - improved, green box now shows
  470711-1-ref.xul - improved, tabs now appear
  676387-1-ref.xul - improved, button text now shows
  checkbox-dynamic-change-ref.xul - improved, checkbox is now visible
  radio-dynamic-change-ref.xul - improved, radio is now visible
  menulist-shrinkwrap-1-ref.xul - unsure, drop down larger with arrow
  menulist-shrinkwrap-2-ref.xul - unsure, text is now visible but button is cut off
  accesskey-ref.xul - unsure, access key indicator (X) is now visible
  tree-row-outline-1-ref.xul - improved, icon now visible in header
  treechildren-padding-percent-1-ref.xul - improved, icon now visible in header
  treetwisty-svg-context-paint-1-ref.xul - improved, icon now visible in header

!= Reftests:

Linux:
  emptytextbox-4.xul - improved, search icon now visible
  tree-row-outline-1.xul - improved, header icon now visible
  tree-row-outline-1-notref.xul - improved, header icon now visible
  treetwisty-svg-context-paint-1-not-ref.xul - improved, header icon now visible
  treetwisty-svg-context-paint-1-ref.xul - improved, header icon now visible
MacOS:
  same as linux
Windows 10:
  same as linux

Comment 8

[Brendan Dahl [:bdahl]]

Found a spot where to log missing files, and found there was already logging there. I went through all the reftest and crashtest logs and found three places that were warning about missing files. Two of them are expected and one is actually a problem with a current test referring to a file that doesn't exist. I filed bug 1562308 for the latter.

David,
Given the above testing in comment #5 and checking for missing files, are you okay with this landing now?

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Marked both patches as review+.

For what it's worth, another technique that I should have mentioned is that you can do a try push or local run with != and == swapped (for the tests that you want images of), so that you get failures reported. You can then look at all the images of those "failures" in reftest-analyzer and see what the tests look like.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Oh, but please do include the fix I suggested for bug 1557383 in the first review.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c68a6b2e0157
Part 1 - Load all XUL reftests with chrome privilege. r=dbaron

Comment 12

[Alexandru Michis [:malexandru]]

Backed out changeset c68a6b2e0157 for causing failures in reftest/content/bugs/272646-1.xul

Backout link: https://hg.mozilla.org/integration/autoland/rev/9525f0cc6ca1bae213239c542b7a6642f05c1f8e

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&searchStr=os%2Cx%2C10.14%2Cquantumrender%2Cshippable%2Copt%2Creftests%2Ctest-macosx1014-64-shippable-qr%2Fopt-reftest-e10s-8%2Cr%28r8%29&fromchange=c68a6b2e01576b844a9b0637b73a501345bb5fef&tochange=9525f0cc6ca1bae213239c542b7a6642f05c1f8e&selectedJob=254368146

Failure log: 02:46:47 INFO - REFTEST TEST-START | chrome://reftest/content/bugs/272646-1.xul == chrome://reftest/content/bugs/272646-1-ref.xul
02:46:47 INFO - REFTEST TEST-LOAD | chrome://reftest/content/bugs/272646-1.xul | 295 / 2062 (14%)
02:46:47 INFO - REFTEST TEST-LOAD | chrome://reftest/content/bugs/272646-1-ref.xul | 295 / 2062 (14%)
02:46:47 INFO - REFTEST TEST-UNEXPECTED-FAIL | chrome://reftest/content/bugs/272646-1.xul == chrome://reftest/content/bugs/272646-1-ref.xul | image comparison, max difference: 255, number of differing pixels: 383

Comment 13

[Brendan Dahl [:bdahl]]

One of the broken tests regressed in between me running on try and autoland. I've file bug 1563331 to track the regression.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18a48e5113cd
Part 1 - Load all XUL reftests with chrome privilege. r=dbaron

Comment 15

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/18a48e5113cd

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/baf1fb2ce286
Part 2 - Load all XUL crashtests with chrome privilege. r=dbaron

Comment 17

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/baf1fb2ce286