1595878 - Identify default preferences and set them earlier in remote agent setup

Status: NEW

(Remote Protocol :: Agent, task, P3)

Description

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

There are some preferences that will be relevant to most use-cases for the Remote Agent. Some must be set before browser start-up (in the profile) whereas others can be set at run-time. The fewer default preferences we have and the earlier the run-time preferences are set, the better, to support easy use of the remote agent by internal tools.

Currently we set some preferences in RemoteAgentClass.listen (see Bug 1553102) -- these can perhaps be moved earlier, to the command-line handler, say. Some of these might be too specific and could move to clients like Puppeteer (see https://github.com/GoogleChrome/puppeteer/issues/5149).

Comment 1

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

I'm going to pause work on this until Bug 1543115 is sorted. All the prefs we need can go in the Puppeteer profile anyway (Bug 1596888), so this is not a blocker.

Based on what I've found in geckodriver, Marionette and Puppeteer, these are the prefs I suggest we include at agent start-up.

// Allow the application to have focus even it runs in the background 
'focusmanager.testmode': true

// Prevent starting into safe mode after application crashes 
'toolkit.startup.max_resumed_crashes': -1

// Disable automatically upgrading Firefox 
// Should be set in profile, too
'app.update.disabledForTesting', true, 

// Increase the APZ content response timeout in tests to 1 minute
// to accomodate slow environments
'apz.content_response_timeout', 60000,

// Do not restore the last open set of tabs if the browser has crashed 
'browser.sessionstore.resume_from_crash', false

// Skip check for default browser on startup
// Should be set in profile, too
'browser.shell.checkDefaultBrowser', false);

// Do not redirect user when a milestone upgrade of Firefox is detected 
'browser.startup.homepage_override.mstone', 'ignore');

// Do not warn when closing all other open tabs 
'browser.tabs.warnOnCloseOtherTabs', false);

// Do not warn when multiple tabs will be opened 
'browser.tabs.warnOnOpen', false);

// Disable the UI tour. 
// Should also be set in profile. 
'browser.uitour.enabled', false, 

// Do not warn on quitting Firefox 
'browser.warnOnQuit', false

These I'm not sure whether to include in the agent, a profile or anywhere at all:

// Do not automatically fill sign-in forms with known usernames and 
// passwords 
'signon.autofillForms', false

// Disable password capture 
'signon.rememberSignons', false

  // Don't unload tabs when available memory is running low 
'browser.tabs.unloadOnLowMemory', false, 

'devtools.console.stdout.chrome', true

// Automatically unload beforeunload alerts 
'dom.disable_beforeunload', true, 

// Don't do network connections for mitm priming 
'security.certerrors.mitm.priming.enabled', false, 

If you have any immediate opinions, please do share, otherwise feel free to clear the n-i.

Comment 2

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

// Disable automatically upgrading Firefox
// Should be set in profile, too
'app.update.disabledForTesting', true,

This absolutely has to be set in the profile. Otherwise a pending update would be installed.

// Do not redirect user when a milestone upgrade of Firefox is detected
'browser.startup.homepage_override.mstone', 'ignore');

Beside that I would also suggest:

"browser.startup.page": 0
"startup.homepage_welcome_url": "about:blank",
"startup.homepage_welcome_url.additional": "",

That ensures that really no additional tabs are opened during the first startup. Only a single tab with about:blank will be presented.

Given our discussion we also want to disable Telemetry support, right? So we should set toolkit.telemetry.server to a fake URL. Maybe our HTTPd.jsm can handle that?

Should we do anything about addons and safebrowsing? Especially the latter would download a lot of data each 15min, which we actually don't want to have for users running their tests with Puppeteer.

I would also suggest the following preferences:

"dom.ipc.reportProcessHangs": False,
"dom.max_chrome_script_run_time": 0, // could go into recommended prefs
"dom.max_script_run_time": 0,  // could go into recommended prefs
"geo.provider.testing": True,
"hangmonitor.timeout": 0,
"network.manage-offline-status": False,
"privacy.trackingprotection.enabled": False

"toolkit.startup.max_resumed_crashes" is currently in recommended preferences, but it should be best set before Firefox gets started. Maybe we should move that. While talking about crashes we shouldn't forget to set the crash environment variables to disable the crash reporter and write minidump files. But this should be a different bug.

Comment 3

[Andreas Tolfsen ❲:ato❳]

The only preference I think we should consider setting at runtime
(i.e. when the remote agent initialises) is focusmanager.testmode
because it directly affects operation if you were to use the CDP
interfaces in a browser-internal context. The remaining preferences
could be set in the profile Puppeteer creates, as you said.

We need to bear in mind that the remote agent may be used outside
an automation context, so the which preferences we set unequivocally
at runtime (initialisation) should not go out of its way to alter
the runtime environment of the user’s browser session. I.e. when
you open the DevTools panel, we don’t set preferences that are
“destructive” to the user’s environment, and we should follow the
same line of thinking here.

We also shouldn’t disable Telemetry because the security team
considers that a security feature.

Comment 4

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

I just stumbled over this particular bug and comment 5, and I'm wondering if we now do the right thing in setting all the recommended preferences with the Remote Agent enabled. See bug 1693803 for the recent change in behavior. CC'ing Julian for information.

We should discuss that in our next triage session, whereby it would be too late for a backout in 91. Also not sure if that would be that easy to do given that lots of other code has been landed.

Comment 5

[Julian Descottes [:jdescottes]]

For marionette the preferences are still set at the same point in time, however for CDP indeed we are now setting the preferences at when calling cdp.start instead of during RemoteAgent init.

I don't think it's worth a backout. The purpose of this bug was to make sure that preferences which are critical to set early are actually set in the profile from Puppeteer. And this was already handled in the PRs which fixed https://github.com/puppeteer/puppeteer/issues/5149

Now that Puppeteer correctly sets those preferences from its profile, I think we should discuss the following items:

• is it still critical to set some preferences early from RemoteAgent?
• should we remove the duplicate prefs between RecommendedPreferences.jsm and https://github.com/puppeteer/puppeteer/blob/d70f60e0619b8659d191fa492e3db4bc221ae982/src/node/Launcher.ts#L433

Comment 6

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

(In reply to Julian Descottes [:jdescottes] from comment #5)

• is it still critical to set some preferences early from RemoteAgent?
• should we remove the duplicate prefs between RecommendedPreferences.jsm and https://github.com/puppeteer/puppeteer/blob/d70f60e0619b8659d191fa492e3db4bc221ae982/src/node/Launcher.ts#L433

Puppeteer should actually set these kind of preferences that require a restart and as such could not be set by the Remote Agent itself. If there is an overlap or duplicates of preferences as set by both we should fix that based on my former sentence.

Setting the recommended preferences when starting a protocol seems to be the correct thing to do. And maybe it would even be better to only do it when there are active connections or a WebDriver session.