1996347 - Disable all "New Tab" features for WebDriver

Status: RESOLVED FIXED

(Remote Protocol :: Agent, task, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

On bug 1944147 the new boolean preference browser.newtabpage.shouldInitialize was added, which we can use to disable all the various features of "about:newtab" and "about:home" at once. It needs to be set to the profile before Firefox starts.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There seems to be issues with loading about:newtab if this preference is set. We do not get to a valid load state and finally time out after 300s

https://treeherder.mozilla.org/logviewer?job_id=533472657&repo=try&task=I1M4QL4eSlmXIZgxj_LNtA.0&lineNumber=55741-55747

1761737578141	Marionette	DEBUG	2 -> [0,45,"WebDriver:Navigate",{"url":"about:newtab"}]
1761737578142	Marionette	TRACE	[12] Received event beforeunload for about:blank
JavaScript error: resource:///modules/AboutNewTabRedirector.sys.mjs, line 544: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIIOService.newChannelFromURIWithLoadInfo]
1761737578254	Marionette	WARN	Ignoring event 'pageshow' because document has an invalid readyState of 'uninitialized'.

Maybe this is related to the visible failure from AboutNewTabRedirector.sys.mjs? Scott, could you please check? Thanks.

Comment 2

[Scott [:thecount] Downe]

Are there steps to test or a patch? Not sure I am following yet.

Just a guess, but based on the way browser.newtabpage.shouldInitialize is setup, it wouldn't surprise me that now any tests loading about:newtab or about:home would never load.

I'm not familiar with AboutNewTabRedirector.sys.mjs, but seemingly if it is relying on some state in newtab after or during initialization, it would, it wouldn't surprise me that it now times out.

I think for me to help, I need a case I can run locally and learn the code path to come up with a solution, probably just a patch? Seemingly you have a test somewhere where you flip browser.newtabpage.shouldInitialize, and then run the test, could I do that locally too?

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The changes that I made can be found by going to the try build from the log link above and then finding the commit on the left side.

Once the patch is applied, run ./mach build and then the following command:

mach marionette-test --gecko-log=- -vv testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py

You can rename all the tests in that file to def tst_ except for test_about_newtab to quicker get into this hang.

There is a test failure as well in browser/components/migration/tests/marionette/test_refresh_firefox.py but lets ignore it for now. Maybe it's somewhat related, and for this test we could just enable all the new tab features. Or if it sounds important I can take a look as well when we figured out the above issue. Thanks!

Comment 4

[Scott [:thecount] Downe]

Attachment: Bug 1996347 - Newtab move feature gate for tests and automation into activitystream

Comment 5

[Scott [:thecount] Downe]

I have a WIP patch, but I don't love how much I have to change to accomplish this, I feel like it's more complex than it needs to be.

I think what's going on is this test:

    def test_about_newtab(self):
        with self.marionette.using_prefs({"browser.newtabpage.enabled": True}):
            self.marionette.navigate("about:newtab")

            self.marionette.navigate(self.test_page_remote)
            self.marionette.find_element(By.ID, "testDiv")

Is turning newtab back on, and relying on the previous turning off feature by feature to work.

Once we stop newtab from initializing at all, it also stops the about:newtab and about:home pages from redirecting to anything. Seemingly.

My WIP patch handles this by forcing a redirect to about:blank, the same way we do for "browser.newtabpage.enabled".

This seems to work, but like I said, it feels overly complex.

I also did some work in the startup cache stuff to be safe, but I wonder if we don't really need to support that use case as the cache is likely never on for new profiles that have initialization turned off?

The other option is we pivot back to a pref that needs to gate every feature on a feature by feature basis, or something like that, but I still worry about human error there, and someone down the road forgets to gate a new feature behind it.

Comment 6

[Scott [:thecount] Downe]

I think I got a better solution.

I switched from "don't initialize newtab" to "don't initialize activitystream"

Which allows the navigator tests to pass because newtab still initializes the basics.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e518714623cf
https://hg.mozilla.org/integration/autoland/rev/40b383e82dcd
Newtab move feature gate for tests and automation into activitystream r=whimboo,home-newtab-reviewers,nbarrett

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/40b383e82dcd

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Scott, thanks a lot for your work! This is a great step forward to reduce possible side-effects especially for new features upcoming in the future.

Comment 10

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #7)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e518714623cf
https://hg.mozilla.org/integration/autoland/rev/40b383e82dcd
Newtab move feature gate for tests and automation into activitystream
r=whimboo,home-newtab-reviewers,nbarrett

Perfherder has detected a awsy performance change from push 40b383e82dcdf9b1936267e9987a275423385e1a.

No action is required from the author; this comment is provided for informational purposes only.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
13%	Images	linux1804-64-shippable-qr	fission tp6	4,521,986.88 -> 3,917,698.24
11%	Images	windows11-64-24h2-shippable	fission tp6	4,481,107.22 -> 3,985,264.48
9%	Images	macosx1470-64-shippable	fission tp6	5,291,074.76 -> 4,816,787.15

Need Help or Information?

If you have any questions, please reach out to [e-mail]. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

Comment 11

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #7)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e518714623cf
https://hg.mozilla.org/integration/autoland/rev/40b383e82dcd
Newtab move feature gate for tests and automation into activitystream
r=whimboo,home-newtab-reviewers,nbarrett

Perfherder has detected a browsertime performance change from push 40b383e82dcdf9b1936267e9987a275423385e1a.

No action is required from the author; this comment is provided for informational purposes only.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
35%	welcome loadtime	macosx1470-64-shippable	cold fission webrender	55.30 -> 35.78	Before/After
10%	welcome loadtime	linux1804-64-shippable-qr	cold fission webrender	67.52 -> 60.65	Before/After
10%	welcome fcp	linux1804-64-shippable-qr	cold fission webrender	119.62 -> 107.97	Before/After

Need Help or Information?

If you have any questions, please reach out to [e-mail]. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.