Commit e7c68804 authored by dfalcantara's avatar dfalcantara Committed by Commit bot

[Document mode] Allow multiple popup windows to appear.

Introduce a queue in TabDelegate for creating multiple Tabs in a row.
This prevents triggering some issue in Android's ActivityManager that
causes it to throw away the tasks for Activities after it launches them,
which in turn causes Android to destroy our Activities.

BUG=498920

Review URL: https://codereview.chromium.org/1229853006

Cr-Commit-Position: refs/heads/master@{#339799}
parent 5444db5d
...@@ -333,7 +333,7 @@ public class ChromeLauncherActivity extends Activity ...@@ -333,7 +333,7 @@ public class ChromeLauncherActivity extends Activity
int shortcutSource = getIntent().getIntExtra( int shortcutSource = getIntent().getIntExtra(
ShortcutHelper.EXTRA_SOURCE, ShortcutSource.UNKNOWN); ShortcutHelper.EXTRA_SOURCE, ShortcutSource.UNKNOWN);
LaunchMetrics.recordHomeScreenLaunchIntoTab(url, shortcutSource); LaunchMetrics.recordHomeScreenLaunchIntoTab(url, shortcutSource);
if (relaunchTask(incognito, url)) return; if (relaunchTask(incognito, url) != Tab.INVALID_TAB_ID) return;
} }
// Create and fire a launch Intent to start a new Task. The old Intent is copied using // Create and fire a launch Intent to start a new Task. The old Intent is copied using
...@@ -450,9 +450,10 @@ public class ChromeLauncherActivity extends Activity ...@@ -450,9 +450,10 @@ public class ChromeLauncherActivity extends Activity
* @param incognito Whether the created document should be incognito. * @param incognito Whether the created document should be incognito.
* @param asyncParams AsyncTabCreationParams to store internally and use later once an intent is * @param asyncParams AsyncTabCreationParams to store internally and use later once an intent is
* received to launch the URL. * received to launch the URL.
* @return ID of the Tab that was launched.
*/ */
@TargetApi(Build.VERSION_CODES.LOLLIPOP) @TargetApi(Build.VERSION_CODES.LOLLIPOP)
public static void launchDocumentInstance( public static int launchDocumentInstance(
Activity activity, boolean incognito, AsyncTabCreationParams asyncParams) { Activity activity, boolean incognito, AsyncTabCreationParams asyncParams) {
assert asyncParams != null; assert asyncParams != null;
...@@ -471,7 +472,8 @@ public class ChromeLauncherActivity extends Activity ...@@ -471,7 +472,8 @@ public class ChromeLauncherActivity extends Activity
if (launchMode == LAUNCH_MODE_RETARGET) { if (launchMode == LAUNCH_MODE_RETARGET) {
assert asyncParams.getWebContents() == null; assert asyncParams.getWebContents() == null;
assert loadUrlParams.getPostData() == null; assert loadUrlParams.getPostData() == null;
if (relaunchTask(incognito, loadUrlParams.getUrl())) return; int relaunchedId = relaunchTask(incognito, loadUrlParams.getUrl());
if (relaunchedId != Tab.INVALID_TAB_ID) return relaunchedId;
} }
// If the new tab is spawned by another tab, record the parent. // If the new tab is spawned by another tab, record the parent.
...@@ -504,6 +506,8 @@ public class ChromeLauncherActivity extends Activity ...@@ -504,6 +506,8 @@ public class ChromeLauncherActivity extends Activity
} else { } else {
fireDocumentIntent(activity, intent, incognito, affiliated, asyncParams); fireDocumentIntent(activity, intent, incognito, affiliated, asyncParams);
} }
return ActivityDelegate.getTabIdFromIntent(intent);
} }
/** /**
...@@ -657,11 +661,11 @@ public class ChromeLauncherActivity extends Activity ...@@ -657,11 +661,11 @@ public class ChromeLauncherActivity extends Activity
* Bring the task matching the given URL to the front if the task is retargetable. * Bring the task matching the given URL to the front if the task is retargetable.
* @param incognito Whether or not the tab is incognito. * @param incognito Whether or not the tab is incognito.
* @param url URL that the tab would have been created for. If null, this param is ignored. * @param url URL that the tab would have been created for. If null, this param is ignored.
* @return Whether the task was successfully brought back. * @return ID of the Tab if it was successfully relaunched, otherwise Tab.INVALID_TAB_ID.
*/ */
@TargetApi(Build.VERSION_CODES.LOLLIPOP) @TargetApi(Build.VERSION_CODES.LOLLIPOP)
private static boolean relaunchTask(boolean incognito, String url) { private static int relaunchTask(boolean incognito, String url) {
if (TextUtils.isEmpty(url)) return false; if (TextUtils.isEmpty(url)) return Tab.INVALID_TAB_ID;
Context context = ApplicationStatus.getApplicationContext(); Context context = ApplicationStatus.getApplicationContext();
ActivityManager manager = ActivityManager manager =
...@@ -681,10 +685,10 @@ public class ChromeLauncherActivity extends Activity ...@@ -681,10 +685,10 @@ public class ChromeLauncherActivity extends Activity
} }
if (!moveToFront(task)) continue; if (!moveToFront(task)) continue;
return true; return id;
} }
return false; return Tab.INVALID_TAB_ID;
} }
/** /**
......
...@@ -16,6 +16,7 @@ import android.os.Build; ...@@ -16,6 +16,7 @@ import android.os.Build;
import android.text.TextUtils; import android.text.TextUtils;
import org.chromium.base.ApplicationStatus; import org.chromium.base.ApplicationStatus;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.Tab; import org.chromium.chrome.browser.Tab;
import org.chromium.chrome.browser.UrlConstants; import org.chromium.chrome.browser.UrlConstants;
import org.chromium.chrome.browser.document.DocumentActivity; import org.chromium.chrome.browser.document.DocumentActivity;
...@@ -204,4 +205,19 @@ public class ActivityDelegate { ...@@ -204,4 +205,19 @@ public class ActivityDelegate {
} }
return false; return false;
} }
}
\ No newline at end of file /** @return Running Activity that owns the given Tab, null if the Activity couldn't be found. */
public static Activity getActivityForTabId(int id) {
if (id == Tab.INVALID_TAB_ID) return null;
for (WeakReference<Activity> ref : ApplicationStatus.getRunningActivities()) {
if (!(ref.get() instanceof ChromeActivity)) continue;
ChromeActivity activity = (ChromeActivity) ref.get();
if (activity == null) continue;
if (activity.getTabModelSelector().getTabById(id) != null) return activity;
}
return null;
}
}
// Copyright 2015 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.tabmodel.document;
import android.app.Activity;
import android.os.Handler;
import android.os.SystemClock;
import org.chromium.base.ThreadUtils;
import org.chromium.chrome.browser.Tab;
import org.chromium.chrome.browser.document.ChromeLauncherActivity;
import org.chromium.chrome.browser.document.DocumentActivity;
import org.chromium.chrome.browser.document.IncognitoDocumentActivity;
import org.chromium.chrome.browser.tabmodel.document.DocumentTabModel.Entry;
import java.util.ArrayList;
/**
* Fires an Intent to launch a new DocumentActivity. Waits for the ActivityManager to acknowledge
* that our task exists before firing the next Intent.
*/
public class AsyncDocumentLauncher {
/**
* Milliseconds to wait for Android to acknowledge that our Activity's task exists.
* This value is empirically defined because Android doesn't let us know when it finishes
* animating Activities, nor does it let us Observe the ActivityManager for when our task
* exists (crbug.com/498920).
*/
private static final int MAX_WAIT_MS = 3000;
/** Milliseconds to wait between polls of the ActivityManager. */
private static final int POLLING_DELAY_MS = 100;
private class LaunchRunnable implements Runnable {
private final int mParentId;
private final AsyncTabCreationParams mAsyncParams;
private int mLaunchedId;
private long mLaunchTimestamp;
public LaunchRunnable(int parentId, AsyncTabCreationParams asyncParams) {
mParentId = parentId;
mAsyncParams = asyncParams;
mLaunchedId = Tab.INVALID_TAB_ID;
}
/** Starts an Activity to with the stored parameters. */
public void launch() {
final Activity parentActivity = ActivityDelegate.getActivityForTabId(mParentId);
mLaunchedId = ChromeLauncherActivity.launchDocumentInstance(
parentActivity, mIsIncognito, mAsyncParams);
mLaunchTimestamp = SystemClock.elapsedRealtime();
run();
}
@Override
public void run() {
// Check if the Activity was already launched.
for (Entry task : mActivityDelegate.getTasksFromRecents(mIsIncognito)) {
if (task.tabId == mLaunchedId) {
finishLaunch();
return;
}
}
if (SystemClock.elapsedRealtime() - mLaunchTimestamp > MAX_WAIT_MS) {
// Check if the launch is taking excessively long. This will likely make the
// previous tab disappear, but it's better than making the user wait.
finishLaunch();
} else {
// Wait a bit longer.
mHandler.postDelayed(this, POLLING_DELAY_MS);
}
}
/** Start up the next tab. */
private void finishLaunch() {
mCurrentRunnable = null;
if (mQueue.size() != 0) {
mCurrentRunnable = mQueue.remove(0);
mCurrentRunnable.launch();
}
}
}
private final boolean mIsIncognito;
private final Handler mHandler;
private final ActivityDelegate mActivityDelegate;
private final ArrayList<LaunchRunnable> mQueue;
private LaunchRunnable mCurrentRunnable;
public AsyncDocumentLauncher(boolean incognito) {
mIsIncognito = incognito;
mHandler = new Handler();
mActivityDelegate = new ActivityDelegate(
DocumentActivity.class, IncognitoDocumentActivity.class);
mQueue = new ArrayList<LaunchRunnable>();
}
/** Enqueues a tab to be launched asynchronously. */
public void enqueueLaunch(int parentId, AsyncTabCreationParams asyncParams) {
ThreadUtils.assertOnUiThread();
LaunchRunnable runnable = new LaunchRunnable(parentId, asyncParams);
if (mCurrentRunnable == null) {
mCurrentRunnable = runnable;
mCurrentRunnable.launch();
} else {
mQueue.add(runnable);
}
}
}
...@@ -11,7 +11,6 @@ import android.net.Uri; ...@@ -11,7 +11,6 @@ import android.net.Uri;
import android.text.TextUtils; import android.text.TextUtils;
import org.chromium.base.ApplicationStatus; import org.chromium.base.ApplicationStatus;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.IntentHandler; import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.Tab; import org.chromium.chrome.browser.Tab;
import org.chromium.chrome.browser.TabState; import org.chromium.chrome.browser.TabState;
...@@ -28,13 +27,12 @@ import org.chromium.content_public.browser.LoadUrlParams; ...@@ -28,13 +27,12 @@ import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.PageTransition; import org.chromium.ui.base.PageTransition;
import java.lang.ref.WeakReference;
/** /**
* Asynchronously creates Tabs by creating/starting up Activities. * Asynchronously creates Tabs by creating/starting up Activities.
*/ */
public class TabDelegate extends TabCreator { public class TabDelegate extends TabCreator {
private boolean mIsIncognito; private final boolean mIsIncognito;
private final AsyncDocumentLauncher mLaunchSerializer;
/** /**
* Creates a TabDelegate. * Creates a TabDelegate.
...@@ -42,6 +40,7 @@ public class TabDelegate extends TabCreator { ...@@ -42,6 +40,7 @@ public class TabDelegate extends TabCreator {
*/ */
public TabDelegate(boolean incognito) { public TabDelegate(boolean incognito) {
mIsIncognito = incognito; mIsIncognito = incognito;
mLaunchSerializer = new AsyncDocumentLauncher(mIsIncognito);
} }
@Override @Override
...@@ -143,11 +142,10 @@ public class TabDelegate extends TabCreator { ...@@ -143,11 +142,10 @@ public class TabDelegate extends TabCreator {
&& asyncParams.getWebContents() != null); && asyncParams.getWebContents() != null);
Context context = ApplicationStatus.getApplicationContext(); Context context = ApplicationStatus.getApplicationContext();
Activity parentActivity = getActivityForTabId(parentId); Activity parentActivity = ActivityDelegate.getActivityForTabId(parentId);
if (FeatureUtilities.isDocumentMode(context)) { if (FeatureUtilities.isDocumentMode(context)) {
ChromeLauncherActivity.launchDocumentInstance( mLaunchSerializer.enqueueLaunch(parentId, asyncParams);
parentActivity, mIsIncognito, asyncParams);
} else { } else {
// TODO(dfalcantara): Is it possible to get rid of this conditional? // TODO(dfalcantara): Is it possible to get rid of this conditional?
int assignedTabId = TabIdManager.getInstance().generateValidId(Tab.INVALID_TAB_ID); int assignedTabId = TabIdManager.getInstance().generateValidId(Tab.INVALID_TAB_ID);
...@@ -173,19 +171,4 @@ public class TabDelegate extends TabCreator { ...@@ -173,19 +171,4 @@ public class TabDelegate extends TabCreator {
IntentHandler.startActivityForTrustedIntent(intent, context); IntentHandler.startActivityForTrustedIntent(intent, context);
} }
} }
/** @return Running Activity that owns the given Tab, null if the Activity couldn't be found. */
private Activity getActivityForTabId(int id) {
if (id == Tab.INVALID_TAB_ID) return null;
for (WeakReference<Activity> ref : ApplicationStatus.getRunningActivities()) {
if (!(ref.get() instanceof ChromeActivity)) continue;
ChromeActivity activity = (ChromeActivity) ref.get();
if (activity == null) continue;
if (activity.getTabModelSelector().getModelForTabId(id) != null) return activity;
}
return null;
}
} }
\ No newline at end of file
...@@ -7,13 +7,15 @@ package org.chromium.chrome.browser; ...@@ -7,13 +7,15 @@ package org.chromium.chrome.browser;
import android.test.suitebuilder.annotation.MediumTest; import android.test.suitebuilder.annotation.MediumTest;
import android.text.TextUtils; import android.text.TextUtils;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.infobar.InfoBar; import org.chromium.chrome.browser.infobar.InfoBar;
import org.chromium.chrome.shell.ChromeShellTestBase; import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.util.FeatureUtilities;
import org.chromium.chrome.test.ChromeActivityTestCaseBase;
import org.chromium.chrome.test.util.TestHttpServerClient; import org.chromium.chrome.test.util.TestHttpServerClient;
import org.chromium.chrome.test.util.browser.TabLoadObserver;
import org.chromium.content.browser.test.util.Criteria; import org.chromium.content.browser.test.util.Criteria;
import org.chromium.content.browser.test.util.CriteriaHelper; import org.chromium.content.browser.test.util.CriteriaHelper;
import org.chromium.content.browser.test.util.TouchCommon; import org.chromium.content.browser.test.util.TouchCommon;
...@@ -22,27 +24,24 @@ import java.util.ArrayList; ...@@ -22,27 +24,24 @@ import java.util.ArrayList;
/** /**
* Tests whether popup windows appear. * Tests whether popup windows appear.
* In document mode, this will end up spawning multiple Activities.
*/ */
public class PopupTest extends ChromeShellTestBase { public class PopupTest extends ChromeActivityTestCaseBase<ChromeActivity> {
private static final String POPUP_HTML_FILENAME = private static final String POPUP_HTML_FILENAME =
TestHttpServerClient.getUrl("chrome/test/data/android/popup_test.html"); TestHttpServerClient.getUrl("chrome/test/data/android/popup_test.html");
private int getNumInfobarsShowing() { public PopupTest() {
return getActivity().getActiveTab().getInfoBarContainer().getInfoBars().size(); super(ChromeActivity.class);
} }
private void loadPageCompletely(Tab tab, String url) throws Exception { private int getNumInfobarsShowing() {
TabLoadObserver observer = new TabLoadObserver(tab, url); return getActivity().getActivityTab().getInfoBarContainer().getInfoBars().size();
assertTrue(CriteriaHelper.pollForCriteria(observer));
waitForActiveShellToBeDoneLoading();
} }
@Override @Override
public void setUp() throws Exception { public void setUp() throws Exception {
super.setUp(); super.setUp();
launchChromeShellWithBlankPage();
waitForActiveShellToBeDoneLoading();
ThreadUtils.runOnUiThread(new Runnable() { ThreadUtils.runOnUiThread(new Runnable() {
@Override @Override
public void run() { public void run() {
...@@ -51,10 +50,15 @@ public class PopupTest extends ChromeShellTestBase { ...@@ -51,10 +50,15 @@ public class PopupTest extends ChromeShellTestBase {
}); });
} }
@Override
public void startMainActivity() throws InterruptedException {
startMainActivityOnBlankPage();
}
@MediumTest @MediumTest
@Feature({"Popup"}) @Feature({"Popup"})
public void testPopupInfobarAppears() throws Exception { public void testPopupInfobarAppears() throws Exception {
loadPageCompletely(getActivity().getActiveTab(), POPUP_HTML_FILENAME); loadUrl(POPUP_HTML_FILENAME);
assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() { assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
@Override @Override
public boolean isSatisfied() { public boolean isSatisfied() {
...@@ -66,16 +70,21 @@ public class PopupTest extends ChromeShellTestBase { ...@@ -66,16 +70,21 @@ public class PopupTest extends ChromeShellTestBase {
@MediumTest @MediumTest
@Feature({"Popup"}) @Feature({"Popup"})
public void testPopupWindowsAppearWhenAllowed() throws Exception { public void testPopupWindowsAppearWhenAllowed() throws Exception {
loadPageCompletely(getActivity().getActiveTab(), POPUP_HTML_FILENAME); boolean isDocumentMode =
FeatureUtilities.isDocumentMode(ApplicationStatus.getApplicationContext());
final TabModelSelector selector = isDocumentMode
? ChromeApplication.getDocumentTabModelSelector()
: getActivity().getTabModelSelector();
loadUrl(POPUP_HTML_FILENAME);
assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() { assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
@Override @Override
public boolean isSatisfied() { public boolean isSatisfied() {
return getNumInfobarsShowing() == 1; return getNumInfobarsShowing() == 1;
} }
})); }));
assertEquals(1, getActivity().getTabModelSelector().getTotalTabCount()); assertEquals(1, selector.getTotalTabCount());
ArrayList<InfoBar> infobars = ArrayList<InfoBar> infobars = selector.getCurrentTab().getInfoBarContainer().getInfoBars();
getActivity().getActiveTab().getInfoBarContainer().getInfoBars();
assertEquals(1, infobars.size()); assertEquals(1, infobars.size());
// Wait until the animations are done, then click the "open popups" button. // Wait until the animations are done, then click the "open popups" button.
...@@ -88,17 +97,29 @@ public class PopupTest extends ChromeShellTestBase { ...@@ -88,17 +97,29 @@ public class PopupTest extends ChromeShellTestBase {
})); }));
TouchCommon.singleClickView(infobar.getContentWrapper().findViewById(R.id.button_primary)); TouchCommon.singleClickView(infobar.getContentWrapper().findViewById(R.id.button_primary));
// Document mode popups appear slowly and sequentially to prevent Android from throwing them
// away, so use a long timeout. http://crbug.com/498920.
assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() { assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
@Override @Override
public boolean isSatisfied() { public boolean isSatisfied() {
if (getNumInfobarsShowing() != 0) return false; if (getNumInfobarsShowing() != 0) return false;
if (getActivity().getTabModelSelector().getTotalTabCount() != 3) return false; return TextUtils.equals("Popup #3", selector.getCurrentTab().getTitle());
if (!TextUtils.equals("Popup #2", getActivity().getActiveTab().getTitle())) { }
return false; }, 7500, CriteriaHelper.DEFAULT_POLLING_INTERVAL));
}
return true; assertEquals(4, selector.getTotalTabCount());
int currentTabId = selector.getCurrentTab().getId();
// Test that revisiting the original page makes popup windows immediately.
loadUrl(POPUP_HTML_FILENAME);
assertTrue(CriteriaHelper.pollForUIThreadCriteria(new Criteria() {
@Override
public boolean isSatisfied() {
if (getNumInfobarsShowing() != 0) return false;
if (selector.getTotalTabCount() != 7) return false;
return TextUtils.equals("Popup #3", selector.getCurrentTab().getTitle());
} }
})); }, 7500, CriteriaHelper.DEFAULT_POLLING_INTERVAL));
assertNotSame(currentTabId, selector.getCurrentTab().getId());
} }
} }
<html> <html>
<head> <head>
<title>Popup test page</title>
<script type="text/javascript"> <script type="text/javascript">
function spawnWindows() { function spawnWindows() {
window.open('data:text/html,<html><head><title>Popup #1</title></head><body /></html>'); window.open('data:text/html,<html><head><title>Popup #1</title></head><body /></html>');
window.open('data:text/html,<html><head><title>Popup #2</title></head><body /></html>'); window.open('data:text/html,<html><head><title>Popup #2</title></head><body /></html>');
window.open('data:text/html,<html><head><title>Popup #3</title></head><body /></html>');
} }
</script> </script>
</head> </head>
<body onload="spawnWindows()"> <body onload="spawnWindows()">
Spawning two popup windows... Spawning three popup windows...
</body> </body>
</html> </html>
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