Commit fa02cbc6 authored by Simeon Anfinrud's avatar Simeon Anfinrud Committed by Commit Bot

[Chromecast] Do not send redundant start()s to Activity.

Although subsequent start() calls to CastWebContentsComponent
correspond to foregrounding a fragment, this has no bearing if
the ActivityDelegate is used for this component.

When CastWebContentsActivity receives a new intent, even with
the same WebContents, it will restart the state machine that
handles the start parameters, which can cause unexpected side
effects.

Bug: Internal b/77459373
Test: cast_shell_junit_tests
Change-Id: I54a2d9628de4e185df0946291d86eb9553889fda
Reviewed-on: https://chromium-review.googlesource.com/1043099Reviewed-by: default avatarStephen Lanham <slan@chromium.org>
Commit-Queue: Simeon Anfinrud <sanfin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555970}
parent 7361dd9a
......@@ -37,6 +37,9 @@ public class CastWebContentsComponent {
*/
public interface OnKeyDownHandler { void onKeyDown(int keyCode); }
/**
* Callback interface for when UI events occur.
*/
public interface SurfaceEventHandler {
void onVisibilityChange(int visibilityType);
boolean consumeGesture(int gestureType);
......@@ -65,9 +68,11 @@ public class CastWebContentsComponent {
void stop(Context context);
}
private class ActivityDelegate implements Delegate {
@VisibleForTesting
class ActivityDelegate implements Delegate {
private static final String TAG = "cr_CastWebContent_AD";
private boolean mEnableTouchInput;
private boolean mStarted = false;
public ActivityDelegate(boolean enableTouchInput) {
mEnableTouchInput = enableTouchInput;
......@@ -75,13 +80,16 @@ public class CastWebContentsComponent {
@Override
public void start(StartParams params) {
if (mStarted) return; // No-op if already started.
if (DEBUG) Log.d(TAG, "start: SHOW_WEB_CONTENT in activity");
startCastActivity(params.context, params.webContents, mEnableTouchInput);
mStarted = true;
}
@Override
public void stop(Context context) {
sendStopWebContentEvent();
mStarted = false;
}
}
......
......@@ -12,7 +12,6 @@ import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import android.app.Activity;
import android.app.Application;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
......@@ -35,33 +34,23 @@ import org.robolectric.annotation.Config;
import org.robolectric.shadows.ShadowActivity;
import org.chromium.base.ContextUtils;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chromecast.shell.CastWebContentsComponent.StartParams;
import org.chromium.content_public.browser.WebContents;
import org.chromium.testing.local.LocalRobolectricTestRunner;
/**
* Tests for CastWebContentsComponent.
*/
@RunWith(LocalRobolectricTestRunner.class)
@Config(manifest = Config.NONE, application = CastWebContentsComponentTest.FakeApplication.class)
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE)
public class CastWebContentsComponentTest {
public static class FakeApplication extends Application {
@Override
protected void attachBaseContext(Context base) {
super.attachBaseContext(base);
ContextUtils.initApplicationContextForTests(this);
}
}
private static final String INSTANCE_ID = "1";
private static final String APP_ID = "app";
private static final int VISIBILITY_PRIORITY = 2;
@Mock
private WebContents mWebContents;
private @Mock WebContents mWebContents;
private Activity mActivity;
private ShadowActivity mShadowActivity;
private StartParams mStartParams;
......@@ -240,4 +229,19 @@ public class CastWebContentsComponentTest {
Assert.assertFalse(component.isStarted());
verify(delegate, times(1)).stop(any(Context.class));
}
@Test
public void testStartActivityDelegateTwiceNoops() {
// Sending focus events to a started Activity is unnecessary because the Activity is always
// in focus, and issues with onNewIntent() and duplicate detection can cause unintended
// side effects.
CastWebContentsComponent component =
new CastWebContentsComponent(INSTANCE_ID, null, null, null, false, false);
component.setDelegate(component.new ActivityDelegate(false));
component.start(mStartParams);
Assert.assertEquals(mShadowActivity.getNextStartedActivity().getComponent().getClassName(),
CastWebContentsActivity.class.getName());
component.start(mStartParams);
Assert.assertNull(mShadowActivity.getNextStartedActivity());
}
}
......@@ -5,9 +5,7 @@
package org.chromium.chromecast.shell;
import android.app.Activity;
import android.app.Application;
import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.net.Uri;
......@@ -21,38 +19,22 @@ import org.mockito.MockitoAnnotations;
import org.robolectric.Robolectric;
import org.robolectric.annotation.Config;
import org.chromium.base.ContextUtils;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.content_public.browser.WebContents;
import org.chromium.testing.local.LocalRobolectricTestRunner;
/**
* Tests for CastWebContentsComponent.
*/
@RunWith(LocalRobolectricTestRunner.class)
@Config(manifest = Config.NONE, application = CastWebContentsComponentTest.FakeApplication.class)
@RunWith(BaseRobolectricTestRunner.class)
@Config(manifest = Config.NONE)
public class CastWebContentsIntentUtilsTest {
public static class FakeApplication extends Application {
@Override
protected void attachBaseContext(Context base) {
super.attachBaseContext(base);
ContextUtils.initApplicationContextForTests(this);
}
}
private static final String INSTANCE_ID = "1";
private static final String EXPECTED_URI = "cast://webcontents/1";
private static final String APP_ID = "app";
private static final int VISIBILITY_PRIORITY = 2;
@Mock
private WebContents mWebContents;
@Mock
private BroadcastReceiver mReceiver;
private @Mock WebContents mWebContents;
private @Mock BroadcastReceiver mReceiver;
private Activity mActivity;
@Before
......
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