Add onbeforeunload and have it call onCancel

[rickycodes]

re: #6489

this makes it so the same RPC error that fires when you cancel will also fire when you close the window.

[whymarrh]

Small nitpick: can we move componentWillUnmount below componentDidMount? (We (c|s)hould have a lint rule maybe)

[Gudahtt]

Incidentally, the e2e test failure may have been fixed in the PR I just merged today. I would suggest rebasing on latest develop.

[whymarrh]

I'll wait for the build to pass before approving but this LGTM

[Gudahtt]

Darn, I guess it didn't fix it after all 😞

I was thinking that one of the calls to closeAllWindowHandlesExcept was causing this issue, and I had removed a few. One of the remaining ones might be the culprit though.

[Gudahtt]

Oh, one difference I just noticed between this and the other beforeunload handlers is that the others record a separate metric event for this scenario, rather than reusing the same function as for the cancel button. That would probably be better to do in this case as well.

[Gudahtt]

On second thought, this behavior should be restricted to the notification UI. I don't think we want closing a fullscreen tab or the popup UI to constitute a rejection (especially not the popup UI, given that it closes when your mouse moves away)

[danjm]

Good point @Gudahtt

@rickycodes See how I restricted this to the notification window in this PR: https://github.com/MetaMask/metamask-extension/pull/6340/files

[danjm]

Removing approval until above comment address, just to prevent accidental merger

[rickycodes]

@danjm @Gudahtt I added the ENVIRONMENT_TYPE_NOTIFICATION check.. didn't seem necessary to check for the removeEventListener since those will just return undefined if the event listener hasn't been added

let me know if this needs anything else

[Gudahtt]

LGTM

[whymarrh]

LGTM, thanks @rickycodes!