Closed Bug 1638502 Opened 10 months ago Closed 10 months ago

[DoH] Persist TRR-selection dry-run result

Categories

(Firefox :: Security, task, P1)

Product:
Firefox
Component:
Security
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 78
Tracking Flags:
Tracking Status
firefox78 --- fixed

People

(Reporter: nhnt11, Assigned: nhnt11)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(2 files)

Bug 1638502 - [DoH] Persist TRR-selection dry-run result. r=valentin!,dragana
47 bytes, text/x-phabricator-request
Details | Review
Bug 1638502 - Clean up DoH doorhanger's TabProgressListener properly. r=johannh!
47 bytes, text/x-phabricator-request
Details | Review

At init-time:

  1. If the dry-run-result pref exists and has a value, commit it to doh-rollout.uri
  2. If not, trigger a dry-run, wait for the result to be available, then commit it.
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/46fe0af8f03d
[DoH] Persist TRR-selection dry-run result. r=valentin,johannh

Backed out changeset 46fe0af8f03d (bug 1638502) for browser_trrSelect.js failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=shippable%2Cmochitest&fromchange=46fe0af8f03dadf116cb3cf8a86377d594b2a7ac&tochange=f4424a8a020d3db672e007f2676acd3111f8038d&selectedTaskRun=XwcVmqrtQ7auEVu1OrZwXw-0

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

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=303527400&repo=autoland&lineNumber=3574

[task 2020-05-24T11:06:37.083Z] 11:06:37     INFO - TEST-START | browser/extensions/doh-rollout/test/browser/browser_trrSelect.js
[task 2020-05-24T11:06:44.762Z] 11:06:44     INFO - TEST-INFO | started process screencapture
[task 2020-05-24T11:06:44.947Z] 11:06:44     INFO - TEST-INFO | screencapture: exit 0
[task 2020-05-24T11:06:44.947Z] 11:06:44     INFO - Buffered messages logged at 11:06:37
[task 2020-05-24T11:06:44.947Z] 11:06:44     INFO - Entering test bound setup
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - Leaving test bound setup
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - Entering test bound testTRRSelect
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - TEST-PASS | browser/extensions/doh-rollout/test/browser/browser_trrSelect.js | Breadcrumb saved. - 
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - TEST-PASS | browser/extensions/doh-rollout/test/browser/browser_trrSelect.js | TRR selection complete. - 
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - Console message: [JavaScript Error: "WebExtension context not found!" {file: "resource://gre/modules/ExtensionParent.jsm" line: 1113}]
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - getContextById@resource://gre/modules/ExtensionParent.jsm:1113:13
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - recvAPICall@resource://gre/modules/ExtensionParent.jsm:968:24
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - _recv@resource://gre/modules/ConduitsChild.jsm:78:20
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - receiveMessage@resource://gre/modules/ConduitsParent.jsm:333:20
[task 2020-05-24T11:06:44.948Z] 11:06:44     INFO - JSActor query*_send@resource://gre/modules/ConduitsChild.jsm:63:11
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - _send@resource://gre/modules/ConduitsChild.jsm:111:18
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - callParentAsyncFunction@resource://gre/modules/ExtensionChild.jsm:956:18
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - callAsyncFunction@resource://gre/modules/ExtensionChild.jsm:720:33
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - stub@resource://gre/modules/Schemas.jsm:2679:30
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - setSetting@moz-extension://282d4621-8431-0746-b8ee-86c480335578/background.js:369:47
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - rememberTRRMode@moz-extension://282d4621-8431-0746-b8ee-86c480335578/background.js:108:19
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - async*setState@moz-extension://282d4621-8431-0746-b8ee-86c480335578/background.js:99:24
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - async*heuristics@moz-extension://282d4621-8431-0746-b8ee-86c480335578/background.js:308:26
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - async*init@moz-extension://282d4621-8431-0746-b8ee-86c480335578/background.js:531:21
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - async*start@moz-extension://282d4621-8431-0746-b8ee-86c480335578/background.js:618:15
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - async*@moz-extension://282d4621-8431-0746-b8ee-86c480335578/background.js:661:17
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - async*@moz-extension://282d4621-8431-0746-b8ee-86c480335578/background.js:670:3
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - 
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - Buffered messages finished
[task 2020-05-24T11:06:44.949Z] 11:06:44     INFO - TEST-UNEXPECTED-FAIL | browser/extensions/doh-rollout/test/browser/browser_trrSelect.js | Uncaught exception - undefined - timed out after 50 tries.
[task 2020-05-24T11:06:44.950Z] 11:06:44     INFO - Leaving test bound testTRRSelect
[task 2020-05-24T11:06:44.950Z] 11:06:44     INFO - GECKO(2586) | MEMORY STAT | vsize 7598MB | residentFast 313MB | heapAllocated 112MB
[task 2020-05-24T11:06:44.950Z] 11:06:44     INFO - TEST-OK | browser/extensions/doh-rollout/test/browser/browser_trrSelect.js | took 7747ms
Flags: needinfo?(nhnt11)

I don't think this failure is related to the patch in this bug. I think this is happening when these tests try to restart add-ons, and there might have been a change under the hood that these tests need to account for... Shane, do you know of any recent changes that might be relevant here?

Bogdan, do you feel ok landing this bug and tracking this issue separately?

Flags: needinfo?(nhnt11)
Flags: needinfo?(mixedpuppy)
Flags: needinfo?(btara)

With some help from #teamaddons, I think I figured this out. New patch coming up, the solution is to wait for heuristics telemetry to show up before restarting the add-on - that way we don't do a reload when heuristics are still running.

Flags: needinfo?(mixedpuppy)
Flags: needinfo?(btara)
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/680c68517121
[DoH] Persist TRR-selection dry-run result. r=valentin,johannh
Depends on: 1640709

Backed out for browser-chrome failures on browser_userInterference.js

backout: https://hg.mozilla.org/integration/autoland/rev/e2117466c528dae8a966cb759860d1609c1a682c

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&searchStr=browser-chrome&revision=680c68517121400c80dc66bcb3b0e444c989f1b0&selectedTaskRun=IVoc8m4-SLaqzyQl0SnmUA-0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=303674986&repo=autoland&lineNumber=4581

[task 2020-05-25T19:32:42.737Z] 19:32:42 INFO - Buffered messages logged at 19:32:37
[task 2020-05-25T19:32:42.737Z] 19:32:42 INFO - TEST-PASS | browser/extensions/doh-rollout/test/browser/browser_userInterference.js | Doorhanger shown pref undefined before user interaction. -
[task 2020-05-25T19:32:42.737Z] 19:32:42 INFO - Buffered messages finished
[task 2020-05-25T19:32:42.737Z] 19:32:42 INFO - TEST-UNEXPECTED-FAIL | browser/extensions/doh-rollout/test/browser/browser_userInterference.js | Uncaught exception - undefined - timed out after 50 tries.
[task 2020-05-25T19:32:42.737Z] 19:32:42 INFO - Leaving test bound testUserInterference
[task 2020-05-25T19:32:42.757Z] 19:32:42 INFO - GECKO(3468) | MEMORY STAT | vsize 19406229MB | vsizeMaxContiguous 65024128MB | residentFast 1290MB
[task 2020-05-25T19:32:42.758Z] 19:32:42 INFO - TEST-OK | browser/extensions/doh-rollout/test/browser/browser_userInterference.js | took 6728ms
[task 2020-05-25T19:32:42.758Z] 19:32:42 INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-05-25T19:32:42.758Z] 19:32:42 INFO - TEST-UNEXPECTED-FAIL | browser/extensions/doh-rollout/test/browser/browser_userInterference.js | Found an unexpected tab at the end of test run: https://example.com/ -
[task 2020-05-25T19:32:42.845Z] 19:32:42 INFO - checking window state

Flags: needinfo?(nhnt11)

Mmmmmm it looks like the extra TRR selection stuff is just about causing waitForCondition to not be waiting long enough for prefs to change. I posted a patch on bug 1607822 that changes all of these instances in doh-rollout tests to use a pref observer instead. Try push on top of that patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e376496ad3363e58742784f7930965abbcd04315

Flags: needinfo?(nhnt11)
Depends on: 1607822

Okay! Upon further digging, I now think I know the real issue: this patch causes browser_userInterference.js to run after browser_trrSelect.js, and the key difference between browser_trrSelect.js and the other tests is that it doesn't wait for the doorhanger to show up.

This is problematic, because the add-on sets up a WebProgress listener to add the doorhanger when it's enabled, but this doesn't get removed when it's disabled.

There are two solutions here:

  1. Ensure that each test consumes all there is to be consumed before finishing.
  2. Make the add-on clean up better when getting disabled.

I'd like to do both but I'm going to prioritize 1 because we need to land this ASAP, and getting disabled is not a real-world use-case that we need to support for system add-ons.

(In reply to Nihanth Subramanya [:nhnt11] from comment #12)

I'd like to do both but I'm going to prioritize 1 because we need to land this ASAP, and getting disabled is not a real-world use-case that we need to support for system add-ons.

Scratch this. Making the doorhanger clean up properly is quick as well as clean. Posted a patch. New try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2ceffd2be33b33bedb339a9cabdb7e10ef40479

That seems to have done the trick! The test passed on 8 passes of the bc6 chunk in that try push.

Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/795f01419fc5
Clean up DoH doorhanger's TabProgressListener properly. r=johannh
https://hg.mozilla.org/integration/autoland/rev/a192fd4ed14b
[DoH] Persist TRR-selection dry-run result. r=valentin,johannh

https://hg.mozilla.org/mozilla-central/rev/795f01419fc5
https://hg.mozilla.org/mozilla-central/rev/a192fd4ed14b

Status: ASSIGNED → RESOLVED
Closed: 10 months ago
status-firefox78: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 78
Depends on: 1643848
You need to log in before you can comment on or make changes to this bug.