Commit 00d453e4 authored by Simeon Anfinrud's avatar Simeon Anfinrud Committed by Commit Bot

[Chromecast] Decouple WebContents life cycle from StartParams.

It's possible for a new Intent with the same WebContents, but
different values for mutable parameters like enableTouchInput.
When this happens, the WebContents would be reinitialized, which
breaks an assertion in WebContentsImpl.

By storing the WebContents in a separate Controller, we allow it
to persist across a reactivation of mStartParamsState where the
new WebContents is equal to the previous instance.

Bug: internal b/120089917
Test: cast_shell_junit_tests
Change-Id: Id9d58b577a887ffff4002ec6fcde578a71752b1f
Reviewed-on: https://chromium-review.googlesource.com/c/1352077
Commit-Queue: Simeon Anfinrud <sanfin@chromium.org>
Reviewed-by: default avatarLuke Halliwell <halliwell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611808}
parent 7edf9a10
...@@ -112,8 +112,10 @@ class CastWebContentsSurfaceHelper { ...@@ -112,8 +112,10 @@ class CastWebContentsSurfaceHelper {
(WebContents webContents) -> MediaSessionImpl.fromWebContents(webContents); (WebContents webContents) -> MediaSessionImpl.fromWebContents(webContents);
Observable<Uri> uriState = mStartParamsState.map(params -> params.uri); Observable<Uri> uriState = mStartParamsState.map(params -> params.uri);
Observable<WebContents> webContentsState = Controller<WebContents> webContentsState = new Controller<>();
mStartParamsState.map(params -> params.webContents); mStartParamsState.map(params -> params.webContents)
.subscribe(Observers.onEnter(webContentsState::set));
mCreatedState.subscribe(Observers.onExit(x -> webContentsState.reset()));
// Receive broadcasts indicating the screen turned off while we have active WebContents. // Receive broadcasts indicating the screen turned off while we have active WebContents.
uriState.subscribe((Uri uri) -> { uriState.subscribe((Uri uri) -> {
...@@ -121,6 +123,7 @@ class CastWebContentsSurfaceHelper { ...@@ -121,6 +123,7 @@ class CastWebContentsSurfaceHelper {
filter.addAction(CastIntents.ACTION_SCREEN_OFF); filter.addAction(CastIntents.ACTION_SCREEN_OFF);
return new LocalBroadcastReceiverScope(filter, (Intent intent) -> { return new LocalBroadcastReceiverScope(filter, (Intent intent) -> {
mStartParamsState.reset(); mStartParamsState.reset();
webContentsState.reset();
maybeFinishLater(handler, () -> finishCallback.accept(uri)); maybeFinishLater(handler, () -> finishCallback.accept(uri));
}); });
}); });
...@@ -137,6 +140,7 @@ class CastWebContentsSurfaceHelper { ...@@ -137,6 +140,7 @@ class CastWebContentsSurfaceHelper {
return; return;
} }
mStartParamsState.reset(); mStartParamsState.reset();
webContentsState.reset();
maybeFinishLater(handler, () -> finishCallback.accept(uri)); maybeFinishLater(handler, () -> finishCallback.accept(uri));
}); });
}); });
......
...@@ -11,6 +11,7 @@ import static org.junit.Assert.assertTrue; ...@@ -11,6 +11,7 @@ import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any; import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
...@@ -168,6 +169,28 @@ public class CastWebContentsSurfaceHelperTest { ...@@ -168,6 +169,28 @@ public class CastWebContentsSurfaceHelperTest {
verify(mWebContentsView).open(webContents2); verify(mWebContentsView).open(webContents2);
} }
@Test
public void testDoesNotRestartWebContentsIfNewStartParamsHasSameWebContents() {
WebContents webContents = mock(WebContents.class);
// Create two StartParams that have the same WebContents but different values.
StartParams params1 = new StartParamsBuilder()
.withId("1")
.withWebContents(webContents)
.enableTouchInput(false)
.build();
StartParams params2 = new StartParamsBuilder()
.withId("1")
.withWebContents(webContents)
.enableTouchInput(true)
.build();
Scope scope = mock(Scope.class);
when(mWebContentsView.open(webContents)).thenReturn(scope);
mSurfaceHelper.onNewStartParams(params1);
// The second StartParams has the same WebContents, so we shouldn't open the scopes again.
mSurfaceHelper.onNewStartParams(params2);
verify(mWebContentsView, times(1)).open(webContents);
}
@Test @Test
public void testIsTouchInputEnabled() { public void testIsTouchInputEnabled() {
assertFalse(mSurfaceHelper.isTouchInputEnabled()); assertFalse(mSurfaceHelper.isTouchInputEnabled());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment