Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Performance

- Speed up touch gesture target detection on deeply nested view hierarchies by hit-testing in local coordinates instead of calling `getLocationOnScreen` per view ([#5595](https://github.com/getsentry/sentry-java/pull/5595))
- Avoid constructing an exception per view when resolving view ids during view-hierarchy and gesture capture ([#5631](https://github.com/getsentry/sentry-java/pull/5631))

## 8.46.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,10 @@ private static ViewHierarchyNode viewToNode(@NotNull final View view) {
node.setType(className);

try {
final String identifier = ViewUtils.getResourceId(view);
node.setIdentifier(identifier);
final @Nullable String identifier = ViewUtils.getResourceIdOrNull(view);
if (identifier != null) {
node.setIdentifier(identifier);
}
} catch (Throwable e) {
// ignored
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.sentry.android.core.internal.gestures;

import android.content.res.Resources;
import android.view.View;
import android.widget.AbsListView;
import android.widget.ScrollView;
Expand Down Expand Up @@ -42,13 +41,12 @@ && isViewScrollable(view, isAndroidXAvailable.getValue())) {
}

private UiElement createUiElement(final @NotNull View targetView) {
try {
final String resourceName = ViewUtils.getResourceId(targetView);
@Nullable String className = ClassUtil.getClassName(targetView);
return new UiElement(targetView, className, resourceName, null, ORIGIN);
} catch (Resources.NotFoundException ignored) {
final @Nullable String resourceName = ViewUtils.getResourceIdOrNull(targetView);
if (resourceName == null) {
return null;
}
@Nullable String className = ClassUtil.getClassName(targetView);
return new UiElement(targetView, className, resourceName, null, ORIGIN);
}

private static boolean isViewTappable(final @NotNull View view) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,32 +150,37 @@ private static final class ViewWithLocation {
* @return human-readable view id
*/
static String getResourceIdWithFallback(final @NotNull View view) {
final int viewId = view.getId();
try {
return getResourceId(view);
} catch (Resources.NotFoundException e) {
final @Nullable String resourceId = getResourceIdOrNull(view);
if (resourceId == null) {
// fall back to hex representation of the id
return "0x" + Integer.toString(viewId, 16);
return "0x" + Integer.toString(view.getId(), 16);
}
return resourceId;
}

/**
* Retrieves the human-readable view id based on {@code view.getContext().getResources()}.
* Retrieves the human-readable view id based on {@code view.getContext().getResources()}, or
* {@code null} when the view has no resource-backed id. Returning {@code null} rather than
* throwing avoids exception-driven control flow on hot, main-thread paths such as view-hierarchy
* snapshots and gesture target resolution.
*
* @param view - the view whose id is being retrieved
* @return human-readable view id
* @throws Resources.NotFoundException in case the view id was not found
* @return human-readable view id, or {@code null} if it cannot be resolved
*/
public static String getResourceId(final @NotNull View view) throws Resources.NotFoundException {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this old method signature was removed so it is a breaking change but not part of the public api

public static @Nullable String getResourceIdOrNull(final @NotNull View view) {
final int viewId = view.getId();
if (viewId == View.NO_ID || isViewIdGenerated(viewId)) {
throw new Resources.NotFoundException();
return null;
}
final Resources resources = view.getContext().getResources();
if (resources != null) {
if (resources == null) {
return "";
}
try {
return resources.getResourceEntryName(viewId);
} catch (Resources.NotFoundException e) {
return null;
}
return "";
}

private static boolean isViewIdGenerated(int id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,85 +11,18 @@ import io.sentry.internal.gestures.UiElement
import io.sentry.util.LazyEvaluator
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import org.junit.runner.RunWith
import org.mockito.kotlin.any
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.doThrow
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever

@RunWith(AndroidJUnit4::class)
class ViewUtilsTest {
@Test
fun `getResourceId returns resourceId when available`() {
val view =
mock<View> {
whenever(it.id).doReturn(0x7f010001)

val context = mock<Context>()
val resources = mock<Resources>()
whenever(resources.getResourceEntryName(it.id)).thenReturn("test_view")
whenever(context.resources).thenReturn(resources)
whenever(it.context).thenReturn(context)
}

assertEquals(ViewUtils.getResourceId(view), "test_view")
}

@Test
fun `getResourceId throws when resource id is not available`() {
val view =
mock<View> {
whenever(it.id).doReturn(View.generateViewId())

val context = mock<Context>()
val resources = mock<Resources>()
whenever(resources.getResourceEntryName(any())).doThrow(Resources.NotFoundException())
whenever(context.resources).thenReturn(resources)
whenever(it.context).thenReturn(context)
}

assertFailsWith<Resources.NotFoundException> { ViewUtils.getResourceId(view) }
}

@Test
fun `when view has no id set, resource name is not looked up `() {
val context = mock<Context>()
val resources = mock<Resources>()
whenever(context.resources).thenReturn(resources)

val view =
mock<View> {
whenever(it.id).doReturn(View.NO_ID)
whenever(it.context).thenReturn(context)
}

assertFailsWith<Resources.NotFoundException> { ViewUtils.getResourceId(view) }
verify(context, never()).resources
}

@Test
fun `when view id is generated, resource name is not looked up `() {
val context = mock<Context>()
val resources = mock<Resources>()
whenever(context.resources).thenReturn(resources)

val view =
mock<View> {
// View.generateViewId() starts with 1
whenever(it.id).doReturn(1)
whenever(it.context).thenReturn(context)
}

assertFailsWith<Resources.NotFoundException> { ViewUtils.getResourceId(view) }
verify(context, never()).resources
}

@Test
fun `findTarget hit-tests children in their own local coordinate space`() {
val child = clickableChild()
Expand Down Expand Up @@ -178,6 +111,65 @@ class ViewUtilsTest {
gestureTargetLocators = listOf(AndroidViewGestureTargetLocator(LazyEvaluator { true }))
}

@Test
fun `getResourceIdOrNull returns resource name when available`() {
val view =
mock<View> {
whenever(it.id).doReturn(0x7f010001)

val context = mock<Context>()
val resources = mock<Resources>()
whenever(resources.getResourceEntryName(it.id)).thenReturn("test_view")
whenever(context.resources).thenReturn(resources)
whenever(it.context).thenReturn(context)
}

assertEquals("test_view", ViewUtils.getResourceIdOrNull(view))
}

@Test
fun `getResourceIdOrNull returns null without throwing for generated id`() {
val context = mock<Context>()
val view =
mock<View> {
// View.generateViewId() starts with 1
whenever(it.id).doReturn(1)
whenever(it.context).thenReturn(context)
}

assertNull(ViewUtils.getResourceIdOrNull(view))
verify(context, never()).resources
}

@Test
fun `getResourceIdOrNull returns null without throwing when view has no id`() {
val context = mock<Context>()
val view =
mock<View> {
whenever(it.id).doReturn(View.NO_ID)
whenever(it.context).thenReturn(context)
}

assertNull(ViewUtils.getResourceIdOrNull(view))
verify(context, never()).resources
}

@Test
fun `getResourceIdOrNull returns null without throwing when resource not found`() {
val view =
mock<View> {
whenever(it.id).doReturn(1234)

val context = mock<Context>()
val resources = mock<Resources>()
whenever(resources.getResourceEntryName(it.id)).thenThrow(Resources.NotFoundException())
whenever(context.resources).thenReturn(resources)
whenever(it.context).thenReturn(context)
}

assertNull(ViewUtils.getResourceIdOrNull(view))
}

@Test
fun `getResourceIdWithFallback falls back to hexadecimal id when resource not found`() {
val view =
Expand Down
Loading