Skip to content

Fix ViewTarget is garbage collected during actions tracking#2608

Merged
ambushwork merged 1 commit into
developfrom
yl/fix-action-tracking-flaky
Apr 23, 2025
Merged

Fix ViewTarget is garbage collected during actions tracking#2608
ambushwork merged 1 commit into
developfrom
yl/fix-action-tracking-flaky

Conversation

@ambushwork

@ambushwork ambushwork commented Apr 17, 2025

Copy link
Copy Markdown
Member

What does this PR do?

Prior to this PR, we were using WeakReference on ViewTarget, since ViewTarget is a local variable only in the scope of findTarget function, after execution, there will be zero reference of it, and if GC occurs, the object in WeakReference will be cleaned.

The correct way is that we should use strong reference of ViewTarget, but inside ViewTarget we hold weakReference of View.

Also a dedicated GC unit test is added to cover this case.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@ambushwork
ambushwork force-pushed the yl/fix-action-tracking-flaky branch 4 times, most recently from 6d435ac to 0168d7d Compare April 18, 2025 09:18
@codecov-commenter

codecov-commenter commented Apr 18, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 84.21053% with 3 lines in your changes missing coverage. Please review.

Project coverage is 70.04%. Comparing base (5506381) to head (cd47cfd).
Report is 135 commits behind head on develop.

Files with missing lines Patch % Lines
...lin/com/datadog/android/rum/tracking/ViewTarget.kt 77.78% 0 Missing and 2 partials ⚠️
...internal/instrumentation/gestures/GesturesUtils.kt 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2608      +/-   ##
===========================================
+ Coverage    69.98%   70.04%   +0.06%     
===========================================
  Files          819      819              
  Lines        30559    30566       +7     
  Branches      5120     5128       +8     
===========================================
+ Hits         21386    21409      +23     
+ Misses        7731     7725       -6     
+ Partials      1442     1432      -10     
Files with missing lines Coverage Δ
...entation/gestures/AndroidActionTrackingStrategy.kt 95.00% <100.00%> (ø)
...ernal/instrumentation/gestures/GesturesListener.kt 94.37% <100.00%> (-0.04%) ⬇️
...internal/instrumentation/gestures/GesturesUtils.kt 85.71% <0.00%> (ø)
...lin/com/datadog/android/rum/tracking/ViewTarget.kt 80.00% <77.78%> (-20.00%) ⬇️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ambushwork
ambushwork marked this pull request as ready for review April 18, 2025 09:56
@ambushwork
ambushwork requested review from a team as code owners April 18, 2025 09:56
@ambushwork
ambushwork force-pushed the yl/fix-action-tracking-flaky branch from 0168d7d to 03eb126 Compare April 22, 2025 11:14
jonathanmos
jonathanmos previously approved these changes Apr 22, 2025
mariusc83
mariusc83 previously approved these changes Apr 22, 2025
@ambushwork
ambushwork dismissed stale reviews from mariusc83 and jonathanmos via cd47cfd April 23, 2025 08:39
@ambushwork
ambushwork force-pushed the yl/fix-action-tracking-flaky branch from 03eb126 to cd47cfd Compare April 23, 2025 08:39
@ambushwork
ambushwork merged commit ca0160c into develop Apr 23, 2025
@ambushwork
ambushwork deleted the yl/fix-action-tracking-flaky branch April 23, 2025 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants