Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8b4942d0 authored by Ben Lin's avatar Ben Lin
Browse files

ReusableWDVH: Do not update the view host of the child if already same. 

The idea of the ReusableWDVH is to quickly swap out App Header<->Handle,
while reusing them without reinflating the views. The way it works is
that upon needing to swap, ReusableWDVH will remove all the child views
and add the new view as its child, removing the need to create a new one
just because the view hierarchy has changed.

However this process is called currently inside
DesktopModeWindowDecoration#relayout, which gets called in numerous
places. Two instances:

1) Dragging the app header while in freeform when not-focused (z-order
changes) 2) Dragging the app handle while in vertical splitscreen and
task is at the bottom (onTaskChange called because of Recents animation
launch)

Both of these cause a new transition change, which calls on a relayout,
in the middle of a gesture. When ReusableWDVH removes the children and
re-adds it back, despite removing/adding the same view (in this case,
the caption bar view), because of the removal, ViewGroup implementation
will cancel the on-going gesture on the caption bar thus stops the
entire gesture altogether. This causes issues for 1) where it brings it
to front focus but no longer draggable until user starts another
gesture, and 2) causes a cancel event to the decor view mid-dragzone
from splitscreen.

Instead, by not removing/re-adding the view if the ViewHost is already
the view's parent, then we can stop the gesture from being cancelled.

As an added note, this is not an issue for the regular drag-to-desktop
use case, since the gesture of that starts in the status bar, in which
the caption bar below it acts as a spy window, so ViewGroup actually
never sees the caption bar as the one actively receiving input; so the
cancel event, despite generated, does not go to the caption bar.

This also addresses an issue where our threshold for launching Recents
is always calculated from Y=0 since we assume it's dragged from the
status bar. In the vertical splitscreen case, it is actually the task's
top-Y coordinate, so we will use that as the offset.

Bug: 380208494
Bug: 391609332
Test: atest, Manual with tangor
Flag: com.android.window.flags.enable_desktop_windowing_scvh_cache_bug_fix
Change-Id: I5a4f1aba6318ea2d5343579125b6d84b5fde380f
parent 85a46c53

libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecorViewModel.java

+5 −1
Original line number Diff line number Diff line
@@ -1502,7 +1502,11 @@ public class DesktopModeWindowDecorViewModel implements WindowDecorViewModel, 
                    // Do not create an indicator at all if we're not past transition height.

                    DisplayLayout layout = mDisplayController

                            .getDisplayLayout(relevantDecor.mTaskInfo.displayId);

                    if (ev.getRawY() < 2 * layout.stableInsets().top

                    // It's possible task is not at the top of the screen (e.g. bottom of vertical

                    // Splitscreen)

                    final int taskTop = relevantDecor.mTaskInfo.configuration.windowConfiguration

                            .getBounds().top;

                    if (ev.getRawY() < 2 * layout.stableInsets().top + taskTop

                            && mMoveToDesktopAnimator == null) {

                        return;

                    }

libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/common/viewhost/ReusableWindowDecorViewHost.kt

+5 −4
Original line number Diff line number Diff line
@@ -50,9 +50,8 @@ class ReusableWindowDecorViewHost( 
    @VisibleForTesting

    val viewHostAdapter: SurfaceControlViewHostAdapter =

        SurfaceControlViewHostAdapter(context, display),

    private val rootView: FrameLayout = FrameLayout(context)

) : WindowDecorViewHost, Warmable {

    @VisibleForTesting val rootView = FrameLayout(context)



    private var currentUpdateJob: Job? = null



    override val surfaceControl: SurfaceControl
@@ -131,8 +130,10 @@ class ReusableWindowDecorViewHost( 
        Trace.beginSection("ReusableWindowDecorViewHost#updateViewHost")

        viewHostAdapter.prepareViewHost(configuration, touchableRegion)

        onDrawTransaction?.let { viewHostAdapter.applyTransactionOnDraw(it) }

        if (view.parent != rootView) {

            rootView.removeAllViews()

            rootView.addView(view)

        }

        viewHostAdapter.updateView(rootView, attrs)

        Trace.endSection()

    }

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/common/viewhost/ReusableWindowDecorViewHostTest.kt

+45 −16
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.testing.TestableLooper 
import android.view.SurfaceControl

import android.view.View

import android.view.WindowManager

import android.widget.FrameLayout

import androidx.test.filters.SmallTest

import com.android.wm.shell.ShellTestCase

import com.google.common.truth.Truth.assertThat
@@ -30,6 +31,9 @@ import kotlinx.coroutines.test.runTest 
import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.Mockito.mock

import org.mockito.Mockito.times

import org.mockito.kotlin.clearInvocations

import org.mockito.kotlin.never

import org.mockito.kotlin.spy

import org.mockito.kotlin.verify
@@ -47,24 +51,46 @@ class ReusableWindowDecorViewHostTest : ShellTestCase() { 
    fun update_differentView_replacesView() = runTest {

        val view = View(context)

        val lp = WindowManager.LayoutParams()

        val reusableVH = createReusableViewHost()

        reusableVH.updateView(view, lp, context.resources.configuration, null)

        val rootView = FrameLayout(context)

        val reusableVH = createReusableViewHost(rootView)

        reusableVH.updateView(view, lp, context.resources.configuration)



        assertThat(reusableVH.rootView.childCount).isEqualTo(1)

        assertThat(reusableVH.rootView.getChildAt(0)).isEqualTo(view)

        assertThat(rootView.childCount).isEqualTo(1)

        assertThat(rootView.getChildAt(0)).isEqualTo(view)



        val newView = View(context)

        val newLp = WindowManager.LayoutParams()

        reusableVH.updateView(newView, newLp, context.resources.configuration, null)

        reusableVH.updateView(newView, newLp, context.resources.configuration)



        assertThat(reusableVH.rootView.childCount).isEqualTo(1)

        assertThat(reusableVH.rootView.getChildAt(0)).isEqualTo(newView)

        assertThat(rootView.childCount).isEqualTo(1)

        assertThat(rootView.getChildAt(0)).isEqualTo(newView)

    }



    @Test

    fun update_sameView_doesNotReplaceView() = runTest {

        val view = View(context)

        val lp = WindowManager.LayoutParams()

        val spyRootView = spy(FrameLayout(context))

        val reusableVH = createReusableViewHost(spyRootView)

        reusableVH.updateView(view, lp, context.resources.configuration)



        verify(spyRootView, times(1)).removeAllViews()

        assertThat(spyRootView.childCount).isEqualTo(1)

        assertThat(spyRootView.getChildAt(0)).isEqualTo(view)



        reusableVH.updateView(view, lp, context.resources.configuration)



        clearInvocations(spyRootView)

        verify(spyRootView, never()).removeAllViews()

        assertThat(spyRootView.childCount).isEqualTo(1)

        assertThat(spyRootView.getChildAt(0)).isEqualTo(view)

    }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun updateView_clearsPendingAsyncJob() = runTest {

        val reusableVH = createReusableViewHost()

        val rootView = FrameLayout(context)

        val reusableVH = createReusableViewHost(rootView)

        val asyncView = View(context)

        val syncView = View(context)

        val asyncAttrs = WindowManager.LayoutParams(100, 100)
@@ -83,7 +109,6 @@ class ReusableWindowDecorViewHostTest : ShellTestCase() { 
            view = syncView,

            attrs = syncAttrs,

            configuration = context.resources.configuration,

            onDrawTransaction = null,

        )



        // Would run coroutine if it hadn't been cancelled.
@@ -91,7 +116,7 @@ class ReusableWindowDecorViewHostTest : ShellTestCase() { 


        assertThat(reusableVH.viewHostAdapter.isInitialized()).isTrue()

        // View host view/attrs should match the ones from the sync call.

        assertThat(reusableVH.rootView.getChildAt(0)).isEqualTo(syncView)

        assertThat(rootView.getChildAt(0)).isEqualTo(syncView)

        assertThat(reusableVH.view()!!.layoutParams.width).isEqualTo(syncAttrs.width)

    }
@@ -118,7 +143,8 @@ class ReusableWindowDecorViewHostTest : ShellTestCase() { 
    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun updateViewAsync_clearsPendingAsyncJob() = runTest {

        val reusableVH = createReusableViewHost()

        val rootView = FrameLayout(context)

        val reusableVH = createReusableViewHost(rootView)



        val view = View(context)

        reusableVH.updateViewAsync(
@@ -136,7 +162,7 @@ class ReusableWindowDecorViewHostTest : ShellTestCase() { 
        advanceUntilIdle()



        assertThat(reusableVH.viewHostAdapter.isInitialized()).isTrue()

        assertThat(reusableVH.rootView.getChildAt(0)).isEqualTo(otherView)

        assertThat(rootView.getChildAt(0)).isEqualTo(otherView)

    }



    @Test
@@ -148,7 +174,6 @@ class ReusableWindowDecorViewHostTest : ShellTestCase() { 
            view = view,

            attrs = WindowManager.LayoutParams(100, 100),

            configuration = context.resources.configuration,

            onDrawTransaction = null,

        )



        val t = mock(SurfaceControl.Transaction::class.java)
@@ -159,19 +184,23 @@ class ReusableWindowDecorViewHostTest : ShellTestCase() { 


    @Test

    fun warmUp_addsRootView() = runTest {

        val reusableVH = createReusableViewHost().apply { warmUp() }

        val rootView = FrameLayout(context)

        val reusableVH = createReusableViewHost(rootView).apply { warmUp() }



        assertThat(reusableVH.viewHostAdapter.isInitialized()).isTrue()

        assertThat(reusableVH.view()).isEqualTo(reusableVH.rootView)

        assertThat(reusableVH.view()).isEqualTo(rootView)

    }



    private fun CoroutineScope.createReusableViewHost() =

    private fun CoroutineScope.createReusableViewHost(

        rootView: FrameLayout = FrameLayout(context)

    ) =

        ReusableWindowDecorViewHost(

            context = context,

            mainScope = this,

            display = context.display,

            id = 1,

            viewHostAdapter = spy(SurfaceControlViewHostAdapter(context, context.display)),

            rootView

        )



    private fun ReusableWindowDecorViewHost.view(): View? = viewHostAdapter.viewHost?.view