Skip to content

[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

Merged
merged 15 commits into from
Sep 2, 2022
Merged
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
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;
Copy link
Contributor

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.

Copy link
Contributor Author

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 where AppStartTrace and other Trace metrics live in, because that would allow AppStartTrace access to some package-private methods that should not be allowed to use apart from testing. I only want AppStartTrace (the consumer of FirstDrawDoneListener) to use public static factory to register and create FirstDrawDoneListener.


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)
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
Copy link
Member

Choose a reason for hiding this comment

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

Can we test that it is registered later on a state change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see a way to do it because View class doesn't have public View.dispatchAttachView() unlike the other listeners we have for ViewTreeObserver (ViewTreeObserver.dispatchOnDraw() and ViewTreeObserver.dispatchOnGlobalLayout()), and although there's a private View.dispatchAttachView() it uses parameter classes that are also not visible to us.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only added @Config(sdk = 26) for this test because it uses reflection, so I don't know if in the future Android will change field names or refactor ViewTreeObserver.

The < API 26 bug is that adding OnDrawListeners to ViewTreeObserver doesn't work, it has nothing to do with removing or the listener itself, so I don't think we need to test unregistering for 2 different versions.

Copy link
Member

Choose a reason for hiding this comment

The 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 View.OnAttachStateChangeListener, that it is unregistered once onViewAttachedToWindow is called. Otherwise, we could change the code to leave it registered and the tests would still pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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: View class doesn't have public, easy to use dispatchXYZ() methods like ViewTreeObserver does.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

@leotianlizhan leotianlizhan Sep 1, 2022

Choose a reason for hiding this comment

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

Added a check at the last line of registerForNextDraw_delaysAddingListenerForAPIsBelow26().

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;
}
}