Closed Bug 1557371 Opened 5 years ago Closed 5 years ago

Load all XUL reftests with system privilege

Categories

(Testing :: Reftest, task)

Version 3
task
Not set
normal

Tracking

(firefox70 fixed)

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: bdahl, Assigned: bdahl)

References

Details

Attachments

(2 files)

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.

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.

See previous commit.

Depends on D33986

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

Attachment #9070306 - Attachment description: Bug 1557371 - Load all XUL reftests with chrome privilege. r=dbaron → Bug 1557371 - Part 1 - Load all XUL reftests with chrome privilege. r=dbaron
Attachment #9071317 - Attachment description: Bug 1557371 - Load all XUL crashtests with chrome privilege. r=dbaron → Bug 1557371 - Part 2 - Load all XUL crashtests with chrome privilege. r=dbaron

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.

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.

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

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?

Flags: needinfo?(dbaron)

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.

Flags: needinfo?(dbaron)

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

Flags: needinfo?(bdahl)
Pushed by bdahl@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c68a6b2e0157 Part 1 - Load all XUL reftests with chrome privilege. r=dbaron

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

Depends on: 1563331

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

Flags: needinfo?(bdahl)
Pushed by bdahl@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/18a48e5113cd Part 1 - Load all XUL reftests with chrome privilege. r=dbaron
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Pushed by bdahl@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/baf1fb2ce286 Part 2 - Load all XUL crashtests with chrome privilege. r=dbaron
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: