fix: don't throw on bad icons in BrowserWindow constructor

[ckerr]

Description of Change

Fixes #27303.

Don't throw exceptions in the BrowserWindow constructor when fed an invalid image.

This change adds a new parameter to TryConvert that specifies what to do on error -- to throw or to warn. The default is to throw.

This change also adds a new protected method SetIconImpl() that is basically the old SetIcon() + that new parameter. This was added instead of appending a switch to SetIcon() because the latter is directly callable as public API and it doesn't make sense to expose this switch. SetIcon() is now a one-liner that calls SetIconImpl() with 'throw' behavior. The BrowserWindow constructor calls SetIconImpl() with 'warn' behavior.

CC @codebytere

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• tests are changed or added
• relevant documentation is changed or added
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: fix regression that crashed Electron when processing an invalid icon.

[MarshallOfSound]

Short term this works and makes sense, I've discussed the underlying issue here with a bunch of people though and we really should find a way to solve this generically for all constructors and all theoretical errors.

The problem is specific to super c++ constructors that are bound to JS constructors. i.e. this case of BaseWindow being "supered" to BrowserWindow. There is no way for the C++ BrowserWindow constructor to know that BaseWindow threw a JS exception and should early terminate, also early termination of a C++ constructor is weird and not easy to follow behavior.

My long term suggestion would be to refactor our gin'ed classes into having an empty constructor, an IsInitialized() check (which should be hard CHECK'ed in the gin converter) and move all the initialization logic to an Initialize method that can return a boolean for success. This would all be hidden from end users (you'd still call new BrowserWindow() but under the hood that should create the new C++ class and call Initialize early terminating at each class layer if the parent class Initialize failed. There are probably flaws in this plan but we should definitely figure out a way to do it.

[release-clerk]

Release Notes Persisted

fix regression that crashed Electron when processing an invalid icon.

[trop]

I was unable to backport this PR to "11-x-y" cleanly;
you will need to perform this backport manually.

[trop]

I have automatically backported this PR to "12-x-y", please check out #27463

[trop]

@ckerr has manually backported this PR to "11-x-y", please check out #27478