Skip to content

Commit

Permalink
Optimize readiness check in ToolbarControlContainer
Browse files Browse the repository at this point in the history
We can:
* Check visibility before calling isReadyForTextureCapture
* Clear isDirty when snapshot is the same

Local tracing says this is about half as expensive but the measurement
is a bit noisy.

Bug: 1063346

(cherry picked from commit cf6d18b)

Change-Id: I21e5ff330b308e600e021ef13ff12bf8096e1c56
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4175875
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1094508}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4219870
Commit-Queue: Sky Malice <skym@chromium.org>
Reviewed-by: Krishna Govind <govind@chromium.org>
Owners-Override: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#910}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
Patrick Noland authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent b910027 commit 19681dd
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -424,11 +424,15 @@ private void onCompositorInMotionChange(Boolean compositorInMotion) {
mControlsToken);
mControlsToken = TokenHolder.INVALID_TOKEN;
}
} else if (super.isDirty() && mToolbar.isReadyForTextureCapture().isReady) {
// Motion is starting, and we don't have a good capture. Lock the controls so that a
// new capture doesn't happen and the old capture is not shown. This can be fixed
// once the motion is over.
if (mControlContainerIsVisibleSupplier.getAsBoolean()) {
} else if (super.isDirty() && mControlContainerIsVisibleSupplier.getAsBoolean()) {
CaptureReadinessResult captureReadinessResult = mToolbar.isReadyForTextureCapture();
if (captureReadinessResult.blockReason
== TopToolbarBlockCaptureReason.SNAPSHOT_SAME) {
setDirtyRectEmpty();
} else if (captureReadinessResult.isReady) {
// Motion is starting, and we don't have a good capture. Lock the controls so
// that a new capture doesn't happen and the old capture is not shown. This can
// be fixed once the motion is over.
mControlsToken =
mBrowserStateBrowserControlsVisibilityDelegate
.showControlsPersistentAndClearOldToken(mControlsToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,4 +351,18 @@ public void testIsDirty_ConstraintsIgnoredOnNativePage() {

Assert.assertTrue(adapter.isDirty());
}

@Test
public void testInMotion_viewNotVisible() {
ToolbarViewResourceAdapter adapter =
new ToolbarViewResourceAdapter(mToolbarContainer, false);
initAdapter(adapter);
Mockito.doReturn(CaptureReadinessResult.readyWithSnapshotDifference(
ToolbarSnapshotDifference.URL_TEXT))
.when(mToolbar)
.isReadyForTextureCapture();
mIsVisible = false;

changeInMotion(true, false);
}
}

0 comments on commit 19681dd

Please sign in to comment.