fix(screenshots): Don't capture zero size screenshots

[krystofwoldrich]

📜 Description

In Android SDK the screenshot function checks for invalid screenshots values, like zero-size screenshots. This was missing in iOS.

💡 Motivation and Context

• feat: Screenshots sentry-react-native#2610 (comment)

💚 How did you test it?

I run the SDK locally with the RN sample app.

📝 Checklist

• I reviewed the submitted code
• I added tests to verify the changes
• I updated the docs if needed
• Review from the native team if needed
• No breaking changes

🔮 Next steps

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1202.17 ms	1238.98 ms	36.81 ms
Size	20.75 KiB	383.57 KiB	362.81 KiB

Baseline results on branch: 8.0.0

Startup times

Revision	Plain	With Sentry	Diff
a9e77dc	1231.94 ms	1254.85 ms	22.91 ms
8361c4c	1204.07 ms	1252.74 ms	48.67 ms
cdf9acd	1219.62 ms	1254.80 ms	35.18 ms
b9c9598	1228.57 ms	1251.10 ms	22.53 ms
034ff5e	1231.22 ms	1255.37 ms	24.14 ms
dd266f5	1224.76 ms	1252.63 ms	27.87 ms
2b42e4e	1212.52 ms	1249.24 ms	36.72 ms
bbf5334	1216.84 ms	1238.82 ms	21.98 ms
fb9f27a	1218.55 ms	1249.17 ms	30.62 ms
b9c9598	1232.86 ms	1253.84 ms	20.98 ms

App size

Revision	Plain	With Sentry	Diff
a9e77dc	20.75 KiB	379.12 KiB	358.36 KiB
8361c4c	20.75 KiB	383.87 KiB	363.12 KiB
cdf9acd	20.75 KiB	383.78 KiB	363.03 KiB
b9c9598	20.75 KiB	383.77 KiB	363.01 KiB
034ff5e	20.75 KiB	383.83 KiB	363.07 KiB
dd266f5	20.75 KiB	383.39 KiB	362.64 KiB
2b42e4e	20.75 KiB	383.89 KiB	363.14 KiB
bbf5334	20.75 KiB	383.37 KiB	362.62 KiB
fb9f27a	20.75 KiB	382.11 KiB	361.36 KiB
b9c9598	20.75 KiB	383.77 KiB	363.01 KiB

Previous results on branch: fix-dont-capture-zero-size-screenshots

Startup times

Revision	Plain	With Sentry	Diff
d91ca50	1229.62 ms	1259.23 ms	29.61 ms
d15071c	1223.67 ms	1234.66 ms	10.99 ms
63b24e1	1261.49 ms	1272.02 ms	10.53 ms
3c09ad9	1194.24 ms	1228.64 ms	34.40 ms

App size

Revision	Plain	With Sentry	Diff
d91ca50	20.75 KiB	374.87 KiB	354.12 KiB
d15071c	20.75 KiB	374.63 KiB	353.88 KiB
63b24e1	20.75 KiB	374.87 KiB	354.12 KiB
3c09ad9	20.75 KiB	374.91 KiB	354.16 KiB

[philipphofmann]

m: Can we please add tests for this new functionality? As this code is not really testable you could extract this code into a method, expose it via a private category that you would have to add similar as we have it, for example, for SentryClient+Private.h, and then call it from tests. I can also push the basic solution to your branch if you want @krystofwoldrich.

[krystofwoldrich]

You can push the solution to this branch and I will work with that. It will definitely help me.

Besides exposing it with the private header. Are there any examples in the test on how to mock functions, likely I will have to mock UIGraphicsGetImageFromCurrentImageContext.

[philipphofmann]

The code was already testable. I added a test.

[philipphofmann]

@krystofwoldrich, does this need to be released quickly or can it wait until 8.0.0, which we are currently preparing?

[krystofwoldrich]

@philipphofmann It can wait.

[philipphofmann]

LGTM

[philipphofmann]

l: If the image has 0 height or width, it's already empty. I think we don't need to check for bytes && bytes.length > 0 here.

[brustolin]

Its not the size of the view that makes the image 0 length, its not being able to render the image.

[philipphofmann]

@krystofwoldrich, what's stopping us from merging this PR?

[codecov]

Codecov Report

Merging #2459 (8b99397) into 8.0.0 (c6fb5a9) will increase coverage by 88.41%.
The diff coverage is 100.00%.

❗ Current head 8b99397 differs from pull request most recent head 15331d3. Consider uploading reports for the commit 15331d3 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           8.0.0    #2459       +/-   ##
==========================================
+ Coverage   0.00%   88.41%   +88.41%     
==========================================
  Files        239      178       -61     
  Lines      22236    15211     -7025     
  Branches    9134     5282     -3852     
==========================================
+ Hits           0    13449    +13449     
+ Misses     22231     1583    -20648     
- Partials       5      179      +174     

Impacted Files	Coverage Δ
Sources/Sentry/SentryScreenshot.m	69.56% <100.00%> (+69.56%)	⬆️
Sources/Sentry/SentryUIApplication.m	0.00% <0.00%> (ø)
...ces/SentryCrash/Recording/SentryCrashReportStore.c
...ash/Recording/Monitors/SentryCrashMonitor_Zombie.c
...ding/Tools/SentryCrashStackCursor_MachineContext.h
...yCrash/Recording/Monitors/SentryCrashMonitorType.h
...es/SentryCrash/Recording/Tools/SentryCrashSysCtl.c
Sources/SentryCrash/Recording/Tools/SentryHook.c
...entryCrash/Recording/Monitors/SentryCrashMonitor.c
...Crash/Reporting/Filters/Tools/SentryCrashVarArgs.h
... and 226 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6fb5a9...15331d3. Read the comment docs.

[krystofwoldrich]

@philipphofmann Nothing, just waiting for CI to finish and it's merged.

[philipphofmann]

Thanks @krystofwoldrich for the fix 👏😀