1608597 - [Nudges] Top sites will conflict with the nudges/tips experiment on 74

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[Drew Willcoxon :adw]

Top sites and quantumbar update1 are scheduled to ship on release in 74, if I'm not mistaken. 74 is also the final release where the nudges/tips experiment will run: https://experimenter.services.mozilla.com/experiments/search-tips-aka-nudges/

That's a problem because both top sites and nudges use restricting providers, and right now we only support one restricting provider at a time. The result is that when the experiment opens the quantumbar view to show a tip on 74, the top sites are also shown (at best) or only the top sites are shown, with no tip at all (at worst).

So we need to fix this for 74 somehow. We might fix it by fixing bug 1607797, and if so, this bug is just a dupe of that bug. I can also imagine a wallpaper/quick fix that isn't blocked on fixing that larger bug.

Comment 1

[Drew Willcoxon :adw]

Attachment: Bug 1608597 - Quantumbar: Replace restricting providers with a priority system.

Marco, could you give me feedback on this please? I replaced isRestricting with getPriority -- i.e., I replaced the binary restricting system with a general priority system. Instead of choosing restricting providers, the providers manager chooses the highest-priority providers.

This doesn't per se fix the problem of multiple restricting providers (bug 1607797) because it just morphs it to a problem of multiple highest-priority providers. But it does fix bug 1608597, by giving the built-in top sites provider a lower priority than extension providers.

I'm not sure how we can/should solve the problem of multiple restricting providers. Taking a step back, the only scenario for that is extension providers because for built-in providers we can/should be careful not to have conflicts. For our own extension providers in experiments, we also won't encounter this problem since we're not planning on running multiple experiments on the same users at once. (But if we were, we could expose priorities directly to extensions, which this patch does not do.) So the real problem is for hypothetical third-party extensions. But for them, what even is a good heuristic for determining how to break ties? It seems to me there isn't one. Only: first or last wins, or even just forget about breaking ties and query them all.

Comment 2

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=84a94c78f174c276f32f247ee8f1cff8b92472a2

Comment 3

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=284cebee2103ef0a86502e0bab892fe8adfd6301

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c516b5d716f1
Quantumbar: Replace restricting providers with a priority system. r=mak

Comment 5

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&revision=c516b5d716f162366856cb16a9d5ef73902bb9ee&selectedJob=285401046&searchStr=Linux%2Cx64%2Cdebug%2CMochitests%2Cwith%2Cfission%2Cenabled%2Ctest-linux64%2Fdebug-mochitest-browser-chrome-fis-e10s-2%2CM-fis%28bc2%29

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=285401046&repo=autoland

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

[task 2020-01-17T17:48:33.147Z] 17:48:33 INFO - TEST-PASS | browser/components/urlbar/tests/browser/browser_search_tips.js | View opened - true == true -
[task 2020-01-17T17:48:33.147Z] 17:48:33 INFO - Buffered messages finished
[task 2020-01-17T17:48:33.148Z] 17:48:33 INFO - TEST-UNEXPECTED-FAIL | browser/components/urlbar/tests/browser/browser_search_tips.js | 8 == 1 - JS frame :: chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_search_tips.js :: checkTip :: line 362
[task 2020-01-17T17:48:33.148Z] 17:48:33 INFO - Stack trace:
[task 2020-01-17T17:48:33.149Z] 17:48:33 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_search_tips.js:checkTip:362
[task 2020-01-17T17:48:33.149Z] 17:48:33 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_search_tips.js:checkTab:406
[task 2020-01-17T17:48:33.150Z] 17:48:33 INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/browser/browser_search_tips.js:oncePerSession:247
[task 2020-01-17T17:48:33.150Z] 17:48:33 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1062
[task 2020-01-17T17:48:33.151Z] 17:48:33 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1097
[task 2020-01-17T17:48:33.151Z] 17:48:33 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:925
[task 2020-01-17T17:48:33.152Z] 17:48:33 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:808
[task 2020-01-17T17:48:33.152Z] 17:48:33 INFO - GECKO(5260) | [Child 5448, Main Thread] WARNING: Trying to request nsIHttpChannel from DocumentChannel, this is likely broken: file /builds/worker/workspace/build/src/netwerk/ipc/DocumentChannel.cpp, line 63
[task 2020-01-17T17:48:33.153Z] 17:48:33 INFO - GECKO(5260) | [Child 5448: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 3 (0x7f48544c5000) [pid = 5448] [serial = 169] [outer = 0x7f48566545c0]

Comment 6

[Drew Willcoxon :adw]

Didn't update the patch for bug 1606909, which landed in the meantime. New patch coming.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2eebd8cbc757
Quantumbar: Replace restricting providers with a priority system. r=mak

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/2eebd8cbc757