-
Notifications
You must be signed in to change notification settings - Fork 624
[Fireperf][AASA] FirstDrawDoneListener
#4041
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
Changes from all commits
9a4596b
9f2e9e5
dd046aa
facf87a
449f611
5025305
a0e8b48
9c30ce7
4e7792b
40cfc29
59a55dd
4ca7a91
c5772b1
3098287
95990a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
// Copyright 2022 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// | ||
// You may obtain a copy of the License at | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
package com.google.firebase.perf.util; | ||
|
||
import android.os.Build; | ||
import android.os.Handler; | ||
import android.os.Looper; | ||
import android.view.View; | ||
import android.view.ViewTreeObserver; | ||
import androidx.annotation.RequiresApi; | ||
import java.util.concurrent.atomic.AtomicReference; | ||
|
||
/** | ||
* OnDrawListener that unregisters itself and invokes callback when the next draw is done. This API | ||
* 16+ implementation is an approximation of the initial display time. {@link | ||
* android.view.Choreographer#postFrameCallback} is an Android API that provides a simpler and more | ||
* accurate initial display time, but it was bugged before API 30, hence we use this backported | ||
* implementation. | ||
*/ | ||
@RequiresApi(Build.VERSION_CODES.JELLY_BEAN) | ||
public class FirstDrawDoneListener implements ViewTreeObserver.OnDrawListener { | ||
private final Handler mainThreadHandler = new Handler(Looper.getMainLooper()); | ||
private final AtomicReference<View> viewReference; | ||
private final Runnable callback; | ||
|
||
/** Registers a post-draw callback for the next draw of a view. */ | ||
public static void registerForNextDraw(View view, Runnable drawDoneCallback) { | ||
final FirstDrawDoneListener listener = new FirstDrawDoneListener(view, drawDoneCallback); | ||
// Handle bug prior to API 26 where OnDrawListener from the floating ViewTreeObserver is not | ||
// merged into the real ViewTreeObserver. | ||
// https://android.googlesource.com/platform/frameworks/base/+/9f8ec54244a5e0343b9748db3329733f259604f3 | ||
if (Build.VERSION.SDK_INT < 26 && !isAliveAndAttached(view)) { | ||
view.addOnAttachStateChangeListener( | ||
new View.OnAttachStateChangeListener() { | ||
@Override | ||
public void onViewAttachedToWindow(View view) { | ||
view.getViewTreeObserver().addOnDrawListener(listener); | ||
view.removeOnAttachStateChangeListener(this); | ||
} | ||
|
||
@Override | ||
public void onViewDetachedFromWindow(View view) { | ||
view.removeOnAttachStateChangeListener(this); | ||
} | ||
}); | ||
} else { | ||
view.getViewTreeObserver().addOnDrawListener(listener); | ||
} | ||
} | ||
|
||
private FirstDrawDoneListener(View view, Runnable callback) { | ||
this.viewReference = new AtomicReference<>(view); | ||
this.callback = callback; | ||
} | ||
|
||
@Override | ||
public void onDraw() { | ||
// Set viewReference to null so any onDraw past the first is a no-op | ||
View view = viewReference.getAndSet(null); | ||
if (view == null) { | ||
return; | ||
} | ||
// OnDrawListeners cannot be removed within onDraw, so we remove it with a | ||
// GlobalLayoutListener | ||
view.getViewTreeObserver() | ||
.addOnGlobalLayoutListener(() -> view.getViewTreeObserver().removeOnDrawListener(this)); | ||
mainThreadHandler.postAtFrontOfQueue(callback); | ||
} | ||
|
||
/** | ||
* Helper to avoid <a | ||
* href="https://android.googlesource.com/platform/frameworks/base/+/9f8ec54244a5e0343b9748db3329733f259604f3">bug | ||
* prior to API 26</a>, where the floating ViewTreeObserver's OnDrawListeners are not merged into | ||
* the real ViewTreeObserver during attach. | ||
* | ||
* @return true if the View is already attached and the ViewTreeObserver is not a floating | ||
* placeholder. | ||
*/ | ||
private static boolean isAliveAndAttached(View view) { | ||
return view.getViewTreeObserver().isAlive() && isAttachedToWindow(view); | ||
} | ||
|
||
/** Backport {@link View#isAttachedToWindow()} which is API 19+ only. */ | ||
private static boolean isAttachedToWindow(View view) { | ||
if (Build.VERSION.SDK_INT >= 19) { | ||
return view.isAttachedToWindow(); | ||
} | ||
return view.getWindowToken() != null; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,181 @@ | ||
// Copyright 2022 Google LLC | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// | ||
// You may obtain a copy of the License at | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
package com.google.firebase.perf.util; | ||
|
||
import static com.google.common.truth.Truth.assertThat; | ||
import static org.mockito.Mockito.inOrder; | ||
import static org.mockito.Mockito.mock; | ||
import static org.mockito.Mockito.never; | ||
import static org.mockito.Mockito.times; | ||
import static org.mockito.Mockito.verify; | ||
import static org.robolectric.Shadows.shadowOf; | ||
|
||
import android.os.Build; | ||
import android.os.Handler; | ||
import android.os.Looper; | ||
import android.view.View; | ||
import android.view.ViewTreeObserver; | ||
import android.view.ViewTreeObserver.OnDrawListener; | ||
import androidx.test.core.app.ApplicationProvider; | ||
import java.lang.reflect.Field; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.concurrent.CopyOnWriteArrayList; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.junit.runner.RunWith; | ||
import org.mockito.InOrder; | ||
import org.robolectric.RobolectricTestRunner; | ||
import org.robolectric.annotation.Config; | ||
import org.robolectric.annotation.LooperMode; | ||
|
||
/** Unit tests for {@link FirstDrawDoneListener}. */ | ||
@RunWith(RobolectricTestRunner.class) | ||
@LooperMode(LooperMode.Mode.PAUSED) | ||
public class FirstDrawDoneListenerTest { | ||
private View testView; | ||
|
||
@Before | ||
public void setUp() { | ||
testView = new View(ApplicationProvider.getApplicationContext()); | ||
} | ||
|
||
@Test | ||
@Config(sdk = 25) | ||
leotianlizhan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
public void registerForNextDraw_delaysAddingListenerForAPIsBelow26() | ||
throws NoSuchFieldException, IllegalAccessException, ClassNotFoundException { | ||
ArrayList<OnDrawListener> mOnDrawListeners = | ||
initViewTreeObserverWithListener(testView.getViewTreeObserver()); | ||
assertThat(mOnDrawListeners.size()).isEqualTo(0); | ||
|
||
// OnDrawListener is not registered, it is delayed for later | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we test that it is registered later on a state change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a way to do it because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to Raymond's ask. Currently, the test does not do much validate the listener registration for API < 26. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Figured out a way to do it through reflection, PTAL @visumickey @raymondlam |
||
FirstDrawDoneListener.registerForNextDraw(testView, () -> {}); | ||
assertThat(mOnDrawListeners.size()).isEqualTo(0); | ||
|
||
// Register listener after the view is attached to a window | ||
List<View.OnAttachStateChangeListener> attachListeners = dispatchAttachedToWindow(testView); | ||
assertThat(mOnDrawListeners.size()).isEqualTo(1); | ||
assertThat(mOnDrawListeners.get(0)).isInstanceOf(FirstDrawDoneListener.class); | ||
assertThat(attachListeners).isEmpty(); | ||
} | ||
|
||
@Test | ||
@Config(sdk = 26) | ||
public void registerForNextDraw_directlyAddsListenerForApi26AndAbove() | ||
throws NoSuchFieldException, IllegalAccessException { | ||
ArrayList<OnDrawListener> mOnDrawListeners = | ||
initViewTreeObserverWithListener(testView.getViewTreeObserver()); | ||
assertThat(mOnDrawListeners.size()).isEqualTo(0); | ||
|
||
// Immediately register an OnDrawListener to ViewTreeObserver | ||
FirstDrawDoneListener.registerForNextDraw(testView, () -> {}); | ||
assertThat(mOnDrawListeners.size()).isEqualTo(1); | ||
assertThat(mOnDrawListeners.get(0)).isInstanceOf(FirstDrawDoneListener.class); | ||
} | ||
|
||
@Test | ||
@Config(sdk = 26) | ||
public void onDraw_postsCallbackToFrontOfQueue() { | ||
Handler handler = new Handler(Looper.getMainLooper()); | ||
Runnable drawDoneCallback = mock(Runnable.class); | ||
Runnable otherCallback = mock(Runnable.class); | ||
InOrder inOrder = inOrder(drawDoneCallback, otherCallback); | ||
|
||
FirstDrawDoneListener.registerForNextDraw(testView, drawDoneCallback); | ||
handler.post(otherCallback); // 3rd in queue | ||
handler.postAtFrontOfQueue(otherCallback); // 2nd in queue | ||
testView.getViewTreeObserver().dispatchOnDraw(); // 1st in queue | ||
verify(drawDoneCallback, never()).run(); | ||
verify(otherCallback, never()).run(); | ||
|
||
// Execute all posted tasks | ||
shadowOf(Looper.getMainLooper()).idle(); | ||
inOrder.verify(drawDoneCallback, times(1)).run(); | ||
inOrder.verify(otherCallback, times(2)).run(); | ||
inOrder.verifyNoMoreInteractions(); | ||
} | ||
|
||
@Test | ||
@Config(sdk = 26) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we have a test for un-registering itself in Config(sdk=25) for both OnAttachStateChangeListener and OnDrawListener? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I only added The < API 26 bug is that adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the < API 26 bug, we should be making sure that we have a test to make sure that if we do register a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see what you mean. That is difficult for the same reason as the other comment: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you use a FakeView or mock the View object? This way you have access to the listeners and can trigger the interfaces directly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a check at the last line of |
||
public void onDraw_unregistersItself_inLayoutChangeListener() | ||
throws NoSuchFieldException, IllegalAccessException { | ||
ArrayList<OnDrawListener> mOnDrawListeners = | ||
initViewTreeObserverWithListener(testView.getViewTreeObserver()); | ||
FirstDrawDoneListener.registerForNextDraw(testView, () -> {}); | ||
assertThat(mOnDrawListeners.size()).isEqualTo(1); | ||
|
||
// Does not remove OnDrawListener before onDraw, even if OnGlobalLayout is triggered | ||
testView.getViewTreeObserver().dispatchOnGlobalLayout(); | ||
assertThat(mOnDrawListeners.size()).isEqualTo(1); | ||
|
||
// Removes OnDrawListener in the next OnGlobalLayout after onDraw | ||
testView.getViewTreeObserver().dispatchOnDraw(); | ||
testView.getViewTreeObserver().dispatchOnGlobalLayout(); | ||
assertThat(mOnDrawListeners.size()).isEqualTo(0); | ||
} | ||
|
||
/** | ||
* Returns ViewTreeObserver.mOnDrawListeners field through reflection. Since reflections are | ||
* employed, prefer to be used in tests with fixed API level using @Config(sdk = X). | ||
* | ||
* @param vto ViewTreeObserver instance to initialize and return the mOnDrawListeners from | ||
*/ | ||
private static ArrayList<OnDrawListener> initViewTreeObserverWithListener(ViewTreeObserver vto) | ||
throws NoSuchFieldException, IllegalAccessException { | ||
// Adding a listener forces ViewTreeObserver.mOnDrawListeners to be initialized and non-null. | ||
OnDrawListener placeHolder = () -> {}; | ||
vto.addOnDrawListener(placeHolder); | ||
vto.removeOnDrawListener(placeHolder); | ||
|
||
// Obtain mOnDrawListeners field through reflection | ||
Field mOnDrawListeners = | ||
android.view.ViewTreeObserver.class.getDeclaredField("mOnDrawListeners"); | ||
mOnDrawListeners.setAccessible(true); | ||
ArrayList<OnDrawListener> listeners = (ArrayList<OnDrawListener>) mOnDrawListeners.get(vto); | ||
assertThat(listeners).isNotNull(); | ||
assertThat(listeners.size()).isEqualTo(0); | ||
return listeners; | ||
} | ||
|
||
/** | ||
* Simulates {@link View}'s dispatchAttachedToWindow() on API 25 using reflection. | ||
* | ||
* <p>This only simulates the part where dispatchAttachedToWindow() notifies the list of {@link | ||
* View.OnAttachStateChangeListener}. | ||
* | ||
* @param view the view in which we are simulating dispatchAttachedToWindow(). | ||
* @return list of {@link View.OnAttachStateChangeListener} from the input {@link View} | ||
*/ | ||
private static List<View.OnAttachStateChangeListener> dispatchAttachedToWindow(View view) | ||
throws ClassNotFoundException, NoSuchFieldException, IllegalAccessException { | ||
assert Build.VERSION.SDK_INT == 25; | ||
Class<?> listenerInfo = Class.forName("android.view.View$ListenerInfo"); | ||
Field mListenerInfo = View.class.getDeclaredField("mListenerInfo"); | ||
mListenerInfo.setAccessible(true); | ||
Object li = mListenerInfo.get(view); | ||
assertThat(li).isNotNull(); | ||
Field mOnAttachStateChangeListeners = | ||
listenerInfo.getDeclaredField("mOnAttachStateChangeListeners"); | ||
mOnAttachStateChangeListeners.setAccessible(true); | ||
CopyOnWriteArrayList<View.OnAttachStateChangeListener> listeners = | ||
(CopyOnWriteArrayList<View.OnAttachStateChangeListener>) | ||
mOnAttachStateChangeListeners.get(li); | ||
assertThat(listeners).isNotNull(); | ||
assertThat(listeners).isNotEmpty(); | ||
for (View.OnAttachStateChangeListener listener : listeners) { | ||
listener.onViewAttachedToWindow(view); | ||
} | ||
return listeners; | ||
} | ||
} |
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.
nit: Is there a better package where we drop this in? Util is too open and overtime this has becomes a drop off of different kind of classes. Since this is in the context of app start measurements, can we find a better place for this class? If it is hard to find, let catch up offline on the package structure.
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.
I don't know where else to drop this in. I don't want to drop it in
/metrics
which is whereAppStartTrace
and other Trace metrics live in, because that would allowAppStartTrace
access to some package-private methods that should not be allowed to use apart from testing. I only wantAppStartTrace
(the consumer ofFirstDrawDoneListener
) to use public static factory to register and createFirstDrawDoneListener
.