Commit 48f918d1 authored by Zhiheng Vincent Li's avatar Zhiheng Vincent Li Committed by Commit Bot

[Chromecast] Do not recreate intent receiver in start

CastWebContentsComponent.start() is called everytime when saying "OKG,
play next" before the media app is stopped by a voice command. Because
we lose the reference to the old intent receiver, the
old one has no chance to unregister from LocalBroadcastManager, which
would cause memory leak.

Change-Id: Ie16fb82c220c48c5410cf57a6d829b2984880d95
Bug: b/73789608
Test: cast_shell_junit_tests
Reviewed-on: https://chromium-review.googlesource.com/933581Reviewed-by: default avatarLuke Halliwell <halliwell@chromium.org>
Commit-Queue: Zhiheng(Vincent) Li <vincentli@google.com>
Cr-Commit-Position: refs/heads/master@{#539904}
parent bc561f41
......@@ -17,6 +17,7 @@ import android.support.v4.content.LocalBroadcastManager;
import org.chromium.base.ContextUtils;
import org.chromium.base.Log;
import org.chromium.base.VisibleForTesting;
import org.chromium.content_public.browser.WebContents;
/**
......@@ -38,7 +39,8 @@ public class CastWebContentsComponent {
*/
public interface OnKeyDownHandler { void onKeyDown(int keyCode); }
private interface Delegate {
@VisibleForTesting
interface Delegate {
void start(Context context, WebContents webContents);
void stop(Context context);
}
......@@ -181,12 +183,11 @@ public class CastWebContentsComponent {
private static final String ACTION_ACTIVITY_STOPPED =
"com.google.android.apps.castshell.intent.action.ACTIVITY_STOPPED";
private final Delegate mDelegate;
private final OnComponentClosedHandler mComponentClosedHandler;
private final OnKeyDownHandler mKeyDownHandler;
private final String mInstanceId;
private Delegate mDelegate;
private Receiver mReceiver;
private String mInstanceId;
private boolean mStarted = false;
public CastWebContentsComponent(String instanceId,
OnComponentClosedHandler onComponentClosedHandler, OnKeyDownHandler onKeyDownHandler,
......@@ -194,6 +195,11 @@ public class CastWebContentsComponent {
mComponentClosedHandler = onComponentClosedHandler;
mKeyDownHandler = onKeyDownHandler;
mInstanceId = instanceId;
if (DEBUG) {
Log.d(TAG,
"New CastWebContentsComponent. Instance ID: " + instanceId + "; isHeadless: "
+ isHeadless + "; enableTouchInput:" + enableTouchInput);
}
if (BuildConfig.DISPLAY_WEB_CONTENTS_IN_SERVICE || isHeadless) {
if (DEBUG) Log.d(TAG, "Creating service delegate...");
mDelegate = new ServiceDelegate();
......@@ -207,31 +213,52 @@ public class CastWebContentsComponent {
}
public void start(Context context, WebContents webContents) {
if (DEBUG) Log.d(TAG, "start with delegate: " + mDelegate.getClass().getSimpleName());
if (DEBUG) {
Log.d(TAG,
"start with delegate: " + mDelegate.getClass().getSimpleName()
+ "; Instance ID: " + mInstanceId);
}
Uri instanceUri = getInstanceUri(mInstanceId);
mReceiver = new Receiver();
IntentFilter filter = new IntentFilter();
filter.addAction(ACTION_ACTIVITY_STOPPED);
filter.addAction(ACTION_KEY_EVENT);
filter.addDataScheme(instanceUri.getScheme());
filter.addDataAuthority(instanceUri.getAuthority(), null);
filter.addDataPath(instanceUri.getPath(), PatternMatcher.PATTERN_LITERAL);
if (DEBUG) Log.d(TAG, "Registering mReceiver");
getLocalBroadcastManager().registerReceiver(mReceiver, filter);
mDelegate.start(context, webContents);
mStarted = true;
if (mReceiver == null) {
mReceiver = new Receiver();
IntentFilter filter = new IntentFilter();
filter.addAction(ACTION_ACTIVITY_STOPPED);
filter.addAction(ACTION_KEY_EVENT);
filter.addDataScheme(instanceUri.getScheme());
filter.addDataAuthority(instanceUri.getAuthority(), null);
filter.addDataPath(instanceUri.getPath(), PatternMatcher.PATTERN_LITERAL);
if (DEBUG) Log.d(TAG, "Registering mReceiver");
getLocalBroadcastManager().registerReceiver(mReceiver, filter);
mDelegate.start(context, webContents);
} else {
if (DEBUG) Log.d(TAG, "Receiver already created.");
}
}
@VisibleForTesting
Receiver getIntentReceiver() {
return mReceiver;
}
@VisibleForTesting
void setDelegate(Delegate delegate) {
mDelegate = delegate;
}
public void stop(Context context) {
if (DEBUG) Log.d(TAG, "stop");
if (mStarted) {
if (DEBUG) {
Log.d(TAG,
"stop with delegate: " + mDelegate.getClass().getSimpleName()
+ "; Instance ID: " + mInstanceId);
}
if (mReceiver != null) {
getLocalBroadcastManager().unregisterReceiver(mReceiver);
if (DEBUG) Log.d(TAG, "Call delegate to stop");
mDelegate.stop(context);
mStarted = false;
mReceiver = null;
}
}
......
......@@ -95,8 +95,8 @@ class CastWebContentsSurfaceHelper {
IntentFilter filter = new IntentFilter();
filter.addAction(CastIntents.ACTION_STOP_WEB_CONTENT);
return new LocalBroadcastReceiverScope(filter, (Intent intent) -> {
Log.d(TAG, "Intent action=" + intent.getAction());
String intentUri = intent.getStringExtra(CastWebContentsComponent.INTENT_EXTRA_URI);
Log.d(TAG, "Intent action=" + intent.getAction() + "; URI=" + intentUri);
if (!uri.toString().equals(intentUri)) {
Log.d(TAG, "Current URI=" + uri + "; intent URI=" + intentUri);
return;
......@@ -140,14 +140,15 @@ class CastWebContentsSurfaceHelper {
@Override
public void run() {
if (currentInstanceId != null && currentInstanceId.equals(mInstanceId)) {
Log.d(TAG, "Finishing.");
if (mShowInFragment) {
Intent in = new Intent();
in.setAction(CastIntents.ACTION_ON_WEB_CONTENT_STOPPED);
in.putExtra(CastWebContentsComponent.INTENT_EXTRA_URI, mUri.toString());
Log.d(TAG, "Sending intent: ON_WEB_CONTENT_STOPPED: URI=" + mUri);
LocalBroadcastManager.getInstance(ContextUtils.getApplicationContext())
.sendBroadcastSync(in);
} else {
Log.d(TAG, "Finishing cast content activity of URI:" + mUri);
mHostActivity.finish();
}
}
......
......@@ -6,7 +6,9 @@ package org.chromium.chromecast.shell;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.eq;
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 android.app.Activity;
......@@ -165,4 +167,23 @@ public class CastWebContentsComponentTest {
verify(mActivity, never()).unbindService(any(ServiceConnection.class));
}
@Test
public void testStartWebContentsComponentMultipleTimes() {
CastWebContentsComponent component =
new CastWebContentsComponent(INSTANCE_ID, null, null, false, false);
CastWebContentsComponent.Delegate delegate = mock(CastWebContentsComponent.Delegate.class);
component.setDelegate(delegate);
component.start(mActivity, mWebContents);
Object receiver1 = component.getIntentReceiver();
Assert.assertNotNull(receiver1);
verify(delegate, times(1)).start(eq(mActivity), eq(mWebContents));
component.start(mActivity, mWebContents);
Object receiver2 = component.getIntentReceiver();
Assert.assertEquals(receiver1, receiver2);
verify(delegate, times(1)).start(any(Context.class), any(WebContents.class));
component.stop(mActivity);
Assert.assertNull(component.getIntentReceiver());
verify(delegate, times(1)).stop(any(Context.class));
}
}
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