Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dart Debug Extension: don't forward events to Chrome that will cause a crash #2179

Merged
merged 3 commits into from
Aug 4, 2023

Conversation

elliette
Copy link
Contributor

@elliette elliette commented Aug 2, 2023

See details in https://bugs.chromium.org/p/chromium/issues/detail?id=1469092

This PR skips forwarding events that will cause a crash to Chrome, and instead displays a warning to the user:

Screenshot 2023-08-02 at 3 18 05 PM

FYI @DanTup

Copy link
Contributor

@annagrin annagrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with minor comments

final chromeVersionMatch =
RegExp('Chrome/([0-9.]+)').firstMatch(window.navigator.userAgent);
final chromeVersion =
chromeVersionMatch == null ? '' : chromeVersionMatch[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can this be chromeVersion = chromeVersionMatch?[0] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, done!

RegExp('Chrome/([0-9.]+)').firstMatch(window.navigator.userAgent);
final chromeVersion =
chromeVersionMatch == null ? '' : chromeVersionMatch[0];
if (chromeVersion?.startsWith('Chrome/115') ?? false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can this be

return chromeVersion?.startsWith('Chrome/115') ?? false;

or maybe

if (chromeVersion != null && chromeVersion.startsWith('Chrome/115')) {
  return true;
}

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@auto-submit
Copy link

auto-submit bot commented Aug 3, 2023

auto label is removed for dart-lang/webdev, pr: 2179, due to - The status or check suite unit_test; windows; Dart main; PKG: dwds; `dart test --total-shards 3 --shard-index 1 --exclude-t... has failed. Please fix the issues identified (or deflake) before re-applying this label.

@elliette elliette merged commit 8c667e8 into dart-lang:master Aug 4, 2023
58 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants