chore: update to electron 13

[deepak1556]

• Build nightly exploration builds

[bpasero]

Re smoke tests, make sure to merge in 4cda850

[deepak1556]

They do fail after taking in the above commit, I will give this a try once internal builds are available for triggering insiders

[deepak1556]

Ready for review, plan is to merge next week to get coverage in July iteration insiders.

[bpasero]

To clarify: we only enable crash reporter now if enable-crash-reporter is set? Whats the idea behind this change?

[bpasero]

Weird that node.js still seems to be at 14.16.0 even though https://github.com/electron/electron/releases/tag/v13.0.0 indicates otherwise.

[deepak1556]

With E13 submitURL cannot be empty when uploadToServer is true, in smoke tests we disable crash reporter via the cli switch https://github.com/microsoft/vscode/blob/main/test/automation/src/code.ts#L143 but still end up calling crashReporter.start. I have cleaned up the logic by moving the deciding conditions earlier so that we make call to the crashReporter if it is disabled.

We only enable crash reporter if enable-crash-reporter is set or crash-reporter-directory argument is provided.

[deepak1556]

Yeah it is a typo in the release notes wrt node version, there is no version change in E13. Always refer to this file https://github.com/electron/electron/blob/13-x-y/DEPS#L19 to confirm.

[bpasero]

But doesn't that mean we have to update all our launch scripts to set enable-crash-reporter if we want to enable it for our users?

[deepak1556]

enable-crash-reporter runtime argument defaults to enabled for all our users. Also there is no change in the conditions for crash reporter enablement in this PR, I have just moved the callsites earlier to avoid unnecessary call to crashreporter API.

Are you thinking of another case that breaks with this change ?

[bpasero]

Sorry, I totally forgot we had this in argv.json.