Skip to content

Commit

Permalink
Revert "Android: Make ThreadChecker optimized away in release builds"
Browse files Browse the repository at this point in the history
Reason for revert: see #100

This reverts commit 7e8a59c.
  • Loading branch information
zakharvoit committed May 9, 2024
1 parent e60c260 commit 6969308
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 33 deletions.
18 changes: 5 additions & 13 deletions base/android/java/src/org/chromium/base/ThreadUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

import org.chromium.base.task.PostTask;
import org.chromium.base.task.TaskTraits;
import org.chromium.build.BuildConfig;

import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
Expand All @@ -33,8 +32,9 @@ public class ThreadUtils {
* A helper object to ensure that interactions with a particular object only happens on a
* particular thread.
*
* <pre>Example:
*
* Example:
* <pre>
* {@code
* class Foo {
* // Valid thread is set during construction here.
* private final ThreadChecker mThreadChecker = new ThreadChecker();
Expand All @@ -43,19 +43,11 @@ public class ThreadUtils {
* mThreadChecker.assertOnValidThread();
* }
* }
* }
* </pre>
*/
// TODO(b/274802355): Add @CheckDiscard once R8 can remove this.
public static class ThreadChecker {
private long mThreadId;

public ThreadChecker() {
resetThreadId();
}

public void resetThreadId() {
mThreadId = BuildConfig.ENABLE_ASSERTS ? Process.myTid() : 0;
}
private final long mThreadId = Process.myTid();

/**
* Asserts that the current thread is the same as the one the ThreadChecker was constructed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
* This class stores relevant metrics from FrameMetrics between the calls to UMA reporting methods.
*/
public class FrameMetricsStore {
private final ThreadChecker mThreadChecker = new ThreadChecker();
// FrameMetricsStore can only be accessed on the handler thread (from the
// JankReportingScheduler.getOrCreateHandler() method). However construction occurs on a
// separate thread so the ThreadChecker is instead constructed later.
private ThreadChecker mThreadChecker;
// An arbitrary value from which to create a trace event async track. The only risk if this
// clashes with another track is trace events will show up on both potentially looking weird in
// the tracing UI. No other issue will occur.
Expand Down Expand Up @@ -85,10 +88,8 @@ public static String scenarioToString(@JankScenario.Type int scenario) {
* checking.
*/
void initialize() {
// FrameMetricsStore can only be accessed on the handler thread (from the
// JankReportingScheduler.getOrCreateHandler() method). However construction occurs on a
// separate thread so the ThreadChecker is instead constructed later.
mThreadChecker.resetThreadId();
assert mThreadChecker == null;
mThreadChecker = new ThreadChecker();
}

/** Records the total draw duration and jankiness for a single frame. */
Expand Down
5 changes: 0 additions & 5 deletions base/android/proguard/chromium_code.flags
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@
<init>();
}

# Required to remove fields until b/274802355 is resolved.
-assumevalues class !cr_allowunused,** {
final org.chromium.base.ThreadUtils$ThreadChecker * return _NONNULL_;
}

# TODO(agrieve): Remove once we start to use Android U SDK.
-dontwarn android.window.BackEvent
-dontwarn android.window.OnBackAnimationCallback
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,6 @@
<init>();
}

# Required to remove fields until b/274802355 is resolved.
-assumevalues class !cr_allowunused,** {
final org.chromium.base.ThreadUtils$ThreadChecker * return _NONNULL_;
}

# TODO(agrieve): Remove once we start to use Android U SDK.
-dontwarn android.window.BackEvent
-dontwarn android.window.OnBackAnimationCallback
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@
<init>();
}

# Required to remove fields until b/274802355 is resolved.
-assumevalues class !cr_allowunused,** {
final org.chromium.base.ThreadUtils$ThreadChecker * return _NONNULL_;
}

# TODO(agrieve): Remove once we start to use Android U SDK.
-dontwarn android.window.BackEvent
-dontwarn android.window.OnBackAnimationCallback
Expand Down

0 comments on commit 6969308

Please sign in to comment.