-
-
Notifications
You must be signed in to change notification settings - Fork 239
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
add multiview helper to make the sentry multiview aware. #2366
base: main
Are you sure you want to change the base?
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2366 +/- ##
==========================================
+ Coverage 84.78% 84.93% +0.14%
==========================================
Files 253 80 -173
Lines 9070 2801 -6269
==========================================
- Hits 7690 2379 -5311
+ Misses 1380 422 -958 ☔ View full report in Codecov by Sentry. |
Android Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e2d89fc | 323.84 ms | 376.23 ms | 52.39 ms |
be173fa | 375.94 ms | 458.36 ms | 82.42 ms |
7b2e0ad | 415.59 ms | 457.26 ms | 41.67 ms |
322aa66 | 284.98 ms | 341.76 ms | 56.78 ms |
eecbbca | 324.37 ms | 352.49 ms | 28.12 ms |
f3a18f2 | 456.29 ms | 504.41 ms | 48.12 ms |
e0ba81f | 390.38 ms | 465.40 ms | 75.02 ms |
33527b4 | 403.58 ms | 507.44 ms | 103.86 ms |
3334ac1 | 303.98 ms | 366.65 ms | 62.67 ms |
5e7abc5 | 360.82 ms | 401.18 ms | 40.37 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e2d89fc | 6.06 MiB | 7.03 MiB | 989.37 KiB |
be173fa | 6.35 MiB | 7.33 MiB | 1005.54 KiB |
7b2e0ad | 6.52 MiB | 7.61 MiB | 1.08 MiB |
322aa66 | 5.94 MiB | 6.92 MiB | 1005.75 KiB |
eecbbca | 5.94 MiB | 6.89 MiB | 975.78 KiB |
f3a18f2 | 6.49 MiB | 7.57 MiB | 1.08 MiB |
e0ba81f | 6.52 MiB | 7.59 MiB | 1.06 MiB |
33527b4 | 6.35 MiB | 7.42 MiB | 1.07 MiB |
3334ac1 | 6.06 MiB | 7.03 MiB | 993.54 KiB |
5e7abc5 | 6.34 MiB | 7.28 MiB | 966.66 KiB |
Previous results on branch: feat/multiview-aware
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
218975a | 445.76 ms | 496.16 ms | 50.40 ms |
cae9153 | 497.60 ms | 574.20 ms | 76.60 ms |
f0e82e4 | 483.52 ms | 570.02 ms | 86.50 ms |
1b36c56 | 458.39 ms | 535.86 ms | 77.47 ms |
179091d | 450.02 ms | 498.72 ms | 48.70 ms |
9e610e4 | 464.50 ms | 501.08 ms | 36.58 ms |
1dab88e | 445.29 ms | 501.21 ms | 55.93 ms |
f6239fb | 466.59 ms | 513.08 ms | 46.49 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
218975a | 6.49 MiB | 7.57 MiB | 1.08 MiB |
cae9153 | 6.49 MiB | 7.57 MiB | 1.08 MiB |
f0e82e4 | 6.49 MiB | 7.57 MiB | 1.08 MiB |
1b36c56 | 6.49 MiB | 7.57 MiB | 1.08 MiB |
179091d | 6.49 MiB | 7.57 MiB | 1.08 MiB |
9e610e4 | 6.49 MiB | 7.57 MiB | 1.08 MiB |
1dab88e | 6.49 MiB | 7.57 MiB | 1.08 MiB |
f6239fb | 6.49 MiB | 7.57 MiB | 1.08 MiB |
iOS Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
8cb6557 | 1265.14 ms | 1266.08 ms | 0.94 ms |
bc29768 | 1247.25 ms | 1268.63 ms | 21.38 ms |
3e5078c | 1239.73 ms | 1248.69 ms | 8.96 ms |
6957bfd | 1283.80 ms | 1289.00 ms | 5.20 ms |
0ceb89c | 1252.02 ms | 1271.78 ms | 19.75 ms |
d883d62 | 1221.39 ms | 1230.18 ms | 8.80 ms |
1d47eb7 | 1212.57 ms | 1222.00 ms | 9.43 ms |
21d4150 | 1252.86 ms | 1280.55 ms | 27.69 ms |
6a5a65d | 1237.22 ms | 1250.29 ms | 13.07 ms |
b0811cc | 1238.23 ms | 1255.82 ms | 17.59 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
8cb6557 | 8.10 MiB | 9.18 MiB | 1.08 MiB |
bc29768 | 8.32 MiB | 9.38 MiB | 1.05 MiB |
3e5078c | 8.28 MiB | 9.34 MiB | 1.06 MiB |
6957bfd | 8.15 MiB | 9.15 MiB | 1020.71 KiB |
0ceb89c | 8.15 MiB | 9.12 MiB | 989.78 KiB |
d883d62 | 8.29 MiB | 9.36 MiB | 1.07 MiB |
1d47eb7 | 8.29 MiB | 9.39 MiB | 1.09 MiB |
21d4150 | 8.16 MiB | 9.17 MiB | 1.01 MiB |
6a5a65d | 8.34 MiB | 9.65 MiB | 1.31 MiB |
b0811cc | 8.32 MiB | 9.38 MiB | 1.06 MiB |
Previous results on branch: feat/multiview-aware
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f6239fb | 1243.48 ms | 1259.36 ms | 15.88 ms |
cae9153 | 1228.17 ms | 1244.92 ms | 16.75 ms |
9e610e4 | 1250.29 ms | 1282.87 ms | 32.59 ms |
1b36c56 | 1244.15 ms | 1268.33 ms | 24.18 ms |
179091d | 1253.28 ms | 1279.98 ms | 26.70 ms |
218975a | 1256.33 ms | 1289.69 ms | 33.36 ms |
f0e82e4 | 1254.55 ms | 1279.82 ms | 25.27 ms |
1dab88e | 1246.98 ms | 1274.31 ms | 27.33 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f6239fb | 8.38 MiB | 9.75 MiB | 1.37 MiB |
cae9153 | 8.38 MiB | 9.75 MiB | 1.37 MiB |
9e610e4 | 8.38 MiB | 9.75 MiB | 1.37 MiB |
1b36c56 | 8.38 MiB | 9.75 MiB | 1.37 MiB |
179091d | 8.38 MiB | 9.75 MiB | 1.37 MiB |
218975a | 8.38 MiB | 9.75 MiB | 1.37 MiB |
f0e82e4 | 8.38 MiB | 9.75 MiB | 1.37 MiB |
1dab88e | 8.38 MiB | 9.75 MiB | 1.37 MiB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure how it's gonna work but any chance we can set up some simple tests for this?
I wanna make sure that this doesn't regress in future flutter versions
I'll put this PR into blocked until we can properly test it. Until then we will rely on docs to keep users informed about what to disable to make multiview work |
I'll add an integration test for this |
📜 Description
Currently, the
Sentry Dart Plugin
does not completely support the new multi-view feature for the web. Especially theSentryScreenshotWidget
,SentryUserInteractionWidget
and theWidgetsBindingIntegration
are incompatible.Therefore we deactivate these features if we detect a
multi-view
application.To find out if
multiViewEnabled
is set in theflutter_bootstrap.js
I check thePlatformDispatcher.instance.implicitView
return value.In a regular non multiview app
PlatformDispatcher.instance.implicitView
returns aFlutterView
object. If you try to callPlatformDispatcher.instance.implicitView
you receivenull
. This is also explained here in the Flutter DocsAn alternative approach I found out, could be accessing the
__flutterState
object and look at the elements. In a regular non multiview application the first element is always ameta
event.For a MultiView App the
__flutterState
only containsflutter-view
elements for the number of active views.In this example, there were two active views:
💡 Motivation and Context
Based on the discussion here I added a MultiViewHelper, which is only active for the Web Platform.
💚 How did you test it?
local
📝 Checklist
sendDefaultPii
is enabled🔮 Next steps