Allow favicon fetching code to fetch CORS-enabled images without tain…

[Slava]

…ing the canvas

Fixes #14742

Submitter Checklist:

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Ran git rebase -i to squash commits (if needed).
• Tagged reviewers and labelled the pull request as needed.
• Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

Reviewer Checklist:

• Request a security/privacy review as needed if one was not already requested.

Tests

• Adequate test coverage exists to prevent regressions
• Tests should be independent and work correctly when run individually or as a suite ref
• New files have MPL2 license header

[riastradh-brave]

If canvas.toDataURL can raise an exception, then as a defensive measure, can we also surround it by try { ... } catch (e) { reject(e) }, or is there a reason not to do that?

[Slava]

@riastradh-brave as far as I can tell, we wouldn't do any further error-handling on the promise anyway. And the thrown error does not block the execution of anything else?

We can put a try-catch for cleaner code, but if an error occurs for whatever new reason we don't have a way to respond anyway?

[avoidwork]

@riastradh-brave as an end user & site developer it's more preferable to enable CORS because a favicon over CDN is pretty common.

[bsclifton]

Fix works great and I think is the appropriate one 😄 I had to check on MDN to make sure I wasn't missing anything

Great job and thanks for the quick turn-around 😄 👍

[bsclifton]

@avoidwork this will be in our next release- which should be happening within 1/1.5 weeks 😄

[avoidwork]

yay! thanks!

[bsclifton]

master 7ddb011
0.24.x 6e2d6e1
0.23.x 7d0bd10

[BrendanEich]

Yeah, the CORS crossOrigin property is not as well known as it should be. Thanks again @avoidwork for reporting and pushing on this.

[riastradh-brave]

I didn't mean using try/catch instead of setting crossOrigin -- rather, in addition to, as a defensive measure in case anything else goes wrong, so that in that case the promise is correctly rejected when it fails. But if there are no other external consequences to failure to reject the promise in this case (I haven't examined what the context is), then maybe it doesn't matter.

[avoidwork]

If it throws now it's an invalid or misconfigured URI, which is a good signal for site devs.

[riastradh-brave]

Got it. Sounds good, then!