Skip to content

GitLab

T
tangled
Commit 1c3e620e authored by Lei Tian's avatar Lei Tian Committed by Commit Bot
Browse files
Options

Reland "Use TabPersistentStoreObserver.onMetadataSavedAsynchronously to... 

Reland "Use TabPersistentStoreObserver.onMetadataSavedAsynchronously to background the Browser Actions tab creation service"

This relands commit 64565fd7.

Original change's description:
> Use TabPersistentStoreObserver.onMetadataSavedAsynchronously to
background the Browser Actions tab creation service
>
> Change the logic to background the Browser Actions tab creation
service
> based on TabPersistentStoreObserver.onMetadataSavedAsynchronously. The
> callback indicates that tab model lists are backuped so the saved tabs
> can be restored succesfully.
>
> Also for the notification when a new tab has not been created (tabId
is
> not available), the Intent to bind the notification will be a VIEW
> intent. This notification is necessary to make the service foreground.
>
> Bug: 766349
> Change-Id: I73d49e2a1e26410b2dd6e51a07f35e9965b9a38a
> Reviewed-on: https://chromium-review.googlesource.com/769852
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Commit-Queue: Lei Tian <ltian@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#517492}

Bug: 766349
Change-Id: Icb35c3b0e3ed2186e48c7f0e77ceb6a07bc1f39f
Reviewed-on: https://chromium-review.googlesource.com/791392Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Commit-Queue: Lei Tian <ltian@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519509}
parent 614ea918
Hide whitespace changes
Inline Side-by-side
Showing with 136 additions and 82 deletions
chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionsService.java
View file @ 1c3e620e
......@@ -14,6 +14,7 @@ import android.os.IBinder;
import android.text.TextUtils;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.Callback;
import org.chromium.base.ContextUtils;
import org.chromium.base.VisibleForTesting;
import org.chromium.base.metrics.RecordHistogram;
......@@ -27,17 +28,20 @@ import org.chromium.chrome.browser.notifications.NotificationBuilderFactory;
import org.chromium.chrome.browser.notifications.NotificationConstants;
import org.chromium.chrome.browser.notifications.NotificationUmaTracker;
import org.chromium.chrome.browser.notifications.channels.ChannelDefinitions;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.EmptyTabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType;
import org.chromium.chrome.browser.tabmodel.TabModelObserver;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorImpl;
import org.chromium.chrome.browser.tabmodel.TabPersistentStore.TabModelSelectorMetadata;
import org.chromium.chrome.browser.tabmodel.TabPersistentStore.TabPersistentStoreObserver;
import org.chromium.chrome.browser.util.IntentUtils;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.common.Referrer;
import org.chromium.ui.widget.Toast;
import java.lang.ref.WeakReference;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
/**
* The foreground service responsible for creating notifications for Browser Actions and keep the
......@@ -71,11 +75,12 @@ public class BrowserActionsService extends Service {
private static int sTitleResId;
private static int sLoadingUrlNum;
private static Set<Integer> sPendingTabIds = new HashSet<Integer>();
private static int sPendingCreatedUrlNum;
private BrowserActionsTabModelSelector mSelector;
private TabModelObserver mObserver;
private BrowserActionsTabModelSelector mBrowserActionsSelector;
private TabModelSelectorImpl mTabbedModeTabModelSelector;
private TabPersistentStoreObserver mObserver;
@Override
public IBinder onBind(Intent intent) {
......@@ -95,74 +100,77 @@ public class BrowserActionsService extends Service {
@Override
public int onStartCommand(Intent intent, int flags, int startId) {
if (TextUtils.equals(intent.getAction(), ACTION_TAB_CREATION_START)) {
sendBrowserActionsNotification(Tab.INVALID_TAB_ID);
createNotification(false, Tab.INVALID_TAB_ID);
String linkUrl = IntentUtils.safeGetStringExtra(intent, EXTRA_LINK_URL);
String url = IntentUtils.safeGetStringExtra(intent, EXTRA_LINK_URL);
String sourcePackageName =
IntentUtils.safeGetStringExtra(intent, EXTRA_SOURCE_PACKAGE_NAME);
Context context = ContextUtils.getApplicationContext();
int tabId = openTabInBackground(linkUrl, sourcePackageName);
updateTabIdForNotification(tabId);
if (tabId != Tab.INVALID_TAB_ID) {
finishTabCreation();
}
Toast.makeText(context, R.string.browser_actions_open_in_background_toast_message,
openTabInBackground(url, sourcePackageName);
Toast.makeText(this, R.string.browser_actions_open_in_background_toast_message,
Toast.LENGTH_SHORT)
.show();
updateNumTabCreatedInBackground();
NotificationUmaTracker.getInstance().onNotificationShown(
NotificationUmaTracker.BROWSER_ACTIONS, ChannelDefinitions.CHANNEL_ID_BROWSER);
} else if (TextUtils.equals(intent.getAction(), ACTION_TAB_CREATION_CHROME_DISPLAYED)) {
setLoadingUrlNum(0);
if (mSelector != null) {
mSelector.getModel(false).removeObserver(mObserver);
}
clearPendingStatus();
removeObserver();
stopForeground(true);
}
// The service will not be restarted if Chrome get killed.
return START_NOT_STICKY;
}
private static void setLoadingUrlNum(int num) {
sLoadingUrlNum = num;
private static void clearPendingStatus() {
sPendingTabIds.clear();
sPendingCreatedUrlNum = 0;
}
private void updateTabIdForNotification(int tabId) {
sendBrowserActionsNotification(tabId);
ContextUtils.getAppSharedPreferences()
.edit()
.putBoolean(PREF_HAS_BROWSER_ACTIONS_NOTIFICATION, true)
.apply();
private void createNotification(boolean isUpdate, int tabId) {
sendBrowserActionsNotification(isUpdate, tabId);
if (isUpdate) {
ContextUtils.getAppSharedPreferences()
.edit()
.putBoolean(PREF_HAS_BROWSER_ACTIONS_NOTIFICATION, true)
.apply();
}
}
private void finishTabCreation() {
if (sLoadingUrlNum > 0) {
sLoadingUrlNum--;
if (sLoadingUrlNum == 0) {
stopForeground(false);
if (mSelector != null) {
mSelector.getModel(false).removeObserver(mObserver);
}
}
private void backgroundServiceIfNecessary() {
if (sPendingTabIds.isEmpty() && sPendingCreatedUrlNum == 0) {
stopForeground(false);
removeObserver();
}
}
private void removeObserver() {
if (mObserver == null) return;
if (mBrowserActionsSelector != null) {
mBrowserActionsSelector.removeTabPersistentStoreObserver(mObserver);
}
if (mTabbedModeTabModelSelector != null) {
mTabbedModeTabModelSelector.removeTabPersistentStoreObserver(mObserver);
}
}
@VisibleForTesting
static boolean isBackgroundService() {
return sLoadingUrlNum == 0;
return sPendingTabIds.isEmpty() && sPendingCreatedUrlNum == 0;
}
private int openTabInBackground(String linkUrl, String sourcePackageName) {
private void openTabInBackground(String linkUrl, String sourcePackageName) {
Referrer referrer = IntentHandler.constructValidReferrerForAuthority(sourcePackageName);
LoadUrlParams loadUrlParams = new LoadUrlParams(linkUrl);
loadUrlParams.setReferrer(referrer);
Tab tab = launchTabInRunningTabbedActivity(loadUrlParams);
if (tab != null) {
sLoadingUrlNum++;
return tab.getId();
int tabId = tab.getId();
assert tabId != Tab.INVALID_TAB_ID;
sPendingTabIds.add(tabId);
createNotification(true, tabId);
return;
}
launchTabInBrowserActionsModel(loadUrlParams);
return Tab.INVALID_TAB_ID;
}
private Tab launchTabInRunningTabbedActivity(LoadUrlParams loadUrlParams) {
......@@ -172,48 +180,61 @@ public class BrowserActionsService extends Service {
ChromeTabbedActivity activity = (ChromeTabbedActivity) ref.get();
if (activity == null) continue;
if (activity.getTabModelSelector() != null) {
Tab tab = activity.getTabModelSelector().openNewTab(
mTabbedModeTabModelSelector = (TabModelSelectorImpl) activity.getTabModelSelector();
mTabbedModeTabModelSelector.addTabPersistentStoreObserver(
getTabPersistentStoreObserver());
Tab tab = mTabbedModeTabModelSelector.openNewTab(
loadUrlParams, TabLaunchType.FROM_BROWSER_ACTIONS, null, false);
assert tab != null;
// TODO(ltian): need to listen to the observer from PersistenceStore when TabState
// is saved to disk. See crbug.com/766349.
tab.addObserver(new EmptyTabObserver() {
@Override
public void onPageLoadFinished(Tab tab) {
finishTabCreation();
tab.removeObserver(this);
}
});
return tab;
}
}
return null;
}
private void launchTabInBrowserActionsModel(LoadUrlParams loadUrlParams) {
mSelector = BrowserActionsTabModelSelector.getInstance();
sLoadingUrlNum++;
private TabPersistentStoreObserver getTabPersistentStoreObserver() {
if (mObserver == null) {
mObserver = new EmptyTabModelObserver() {
mObserver = new TabPersistentStoreObserver() {
@Override
public void didAddTab(Tab tab, TabLaunchType type) {
assert mSelector != null;
if (!mSelector.isTabStateInitialized()) return;
assert tab != null;
finishTabCreation();
public void onMetadataSavedAsynchronously(
TabModelSelectorMetadata modelSelectorMetadata) {
removeSavedTabs(modelSelectorMetadata.normalModelMetadata.ids);
backgroundServiceIfNecessary();
}
};
}
mSelector.getModel(false).addObserver(mObserver);
mSelector.openNewTab(loadUrlParams);
return mObserver;
}
private void launchTabInBrowserActionsModel(LoadUrlParams loadUrlParams) {
mBrowserActionsSelector = BrowserActionsTabModelSelector.getInstance();
mBrowserActionsSelector.addTabPersistentStoreObserver(getTabPersistentStoreObserver());
Callback<Integer> tabCreatedCallback = new Callback<Integer>() {
@Override
public void onResult(Integer tabId) {
// Service has been changed to background by opening Chrome so nothing needs to
// update.
if (sPendingCreatedUrlNum == 0) return;
sPendingCreatedUrlNum--;
if (tabId != Tab.INVALID_TAB_ID) sPendingTabIds.add(tabId);
backgroundServiceIfNecessary();
createNotification(true, Tab.INVALID_TAB_ID);
}
};
sPendingCreatedUrlNum++;
mBrowserActionsSelector.openNewTab(loadUrlParams, tabCreatedCallback);
}
private void removeSavedTabs(List<Integer> savedTabIds) {
for (Integer tabId : savedTabIds) sPendingTabIds.remove(tabId);
}
private void sendBrowserActionsNotification(int tabId) {
ChromeNotificationBuilder builder = createNotificationBuilder(tabId);
private void sendBrowserActionsNotification(boolean isUpdate, int tabId) {
ChromeNotificationBuilder builder = createNotificationBuilder(isUpdate, tabId);
startForeground(NotificationConstants.NOTIFICATION_ID_BROWSER_ACTIONS, builder.build());
}
private ChromeNotificationBuilder createNotificationBuilder(int tabId) {
private ChromeNotificationBuilder createNotificationBuilder(boolean isUpdate, int tabId) {
ChromeNotificationBuilder builder =
NotificationBuilderFactory
.createChromeNotificationBuilder(
......@@ -222,21 +243,32 @@ public class BrowserActionsService extends Service {
.setLocalOnly(true)
.setAutoCancel(true)
.setContentText(this.getString(R.string.browser_actions_notification_text));
if (hasBrowserActionsNotification()) {
sTitleResId = R.string.browser_actions_multi_links_open_notification_title;
} else {
sTitleResId = R.string.browser_actions_single_link_open_notification_title;
RecordUserAction.record("BrowserActions.TabOpenedNotificationCreated");
}
sTitleResId = getNotificationTitleId(isUpdate);
builder.setContentTitle(this.getString(sTitleResId));
sNotificationIntent = buildNotificationIntent(tabId);
sNotificationIntent = buildNotificationIntent(isUpdate, tabId);
PendingIntent notifyPendingIntent = PendingIntent.getActivity(
this, 0, sNotificationIntent, PendingIntent.FLAG_UPDATE_CURRENT);
builder.setContentIntent(notifyPendingIntent);
return builder;
}
private Intent buildNotificationIntent(int tabId) {
private int getNotificationTitleId(boolean isUpdate) {
if (isUpdate) {
assert sTitleResId != 0;
} else if (hasBrowserActionsNotification()) {
sTitleResId = R.string.browser_actions_multi_links_open_notification_title;
} else {
sTitleResId = R.string.browser_actions_single_link_open_notification_title;
}
return sTitleResId;
}
/**
* TODO(ltian:) fix ChromeTabbedActivity.processUrlViewIntent to handle Intent after browser
* actions tab merging and use Tab.createBringTabToFrontIntent to create Intent for single url
* in case of browser actions tab merging.
*/
private Intent buildNotificationIntent(boolean isUpdate, int tabId) {
boolean multipleUrls = hasBrowserActionsNotification();
Intent intent;
if (!multipleUrls && tabId != Tab.INVALID_TAB_ID) {
......@@ -245,7 +277,9 @@ public class BrowserActionsService extends Service {
intent = new Intent(this, ChromeLauncherActivity.class);
IntentHandler.addTrustedIntentExtras(intent);
}
intent.putExtra(EXTRA_IS_SINGLE_URL, !multipleUrls);
if (isUpdate) {
intent.putExtra(EXTRA_IS_SINGLE_URL, !multipleUrls);
}
return intent;
}
......@@ -271,7 +305,7 @@ public class BrowserActionsService extends Service {
public static void onTabbedModeForegrounded() {
// If Chrome is shown, force the foreground service to be killed so notification bound to it
// will be dismissed.
if (sLoadingUrlNum != 0) {
if (!sPendingTabIds.isEmpty() || sPendingCreatedUrlNum > 0) {
Context context = ContextUtils.getApplicationContext();
Intent intent = new Intent(context, BrowserActionsService.class);
intent.setAction(ACTION_TAB_CREATION_CHROME_DISPLAYED);
......
chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionsTabModelSelector.java
View file @ 1c3e620e
......@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.browseractions;
import android.os.AsyncTask;
import org.chromium.base.Callback;
import org.chromium.base.ThreadUtils;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.browseractions.BrowserActionsTabCreatorManager.BrowserActionsTabCreator;
......@@ -126,11 +127,12 @@ public class BrowserActionsTabModelSelector
/**
* Creates a new Tab with given url in Browser Actions tab model.
* @param loadUrlParams The url params to be opened.
* @param tabCreatedCallback The {@link Callback} to run when tab is created.
*/
public void openNewTab(LoadUrlParams loadUrlParams) {
public void openNewTab(LoadUrlParams loadUrlParams, Callback<Integer> tabCreatedCallback) {
// If tab model is restored, directly create a new tab.
if (isTabStateInitialized()) {
createNewTab(loadUrlParams);
createNewTab(loadUrlParams, tabCreatedCallback);
return;
}
if (mTabCreationRunnable == null) {
......@@ -138,7 +140,7 @@ public class BrowserActionsTabModelSelector
@Override
public void run() {
for (int i = 0; i < mPendingUrls.size(); i++) {
createNewTab(mPendingUrls.get(i));
createNewTab(mPendingUrls.get(i), tabCreatedCallback);
}
mPendingUrls.clear();
}
......@@ -156,8 +158,9 @@ public class BrowserActionsTabModelSelector
mPendingUrls.add(loadUrlParams);
}
private void createNewTab(LoadUrlParams params) {
openNewTab(params, TabLaunchType.FROM_BROWSER_ACTIONS, null, false);
private void createNewTab(LoadUrlParams params, Callback<Integer> tabCreatedCallback) {
Tab tab = openNewTab(params, TabLaunchType.FROM_BROWSER_ACTIONS, null, false);
tabCreatedCallback.onResult(tab.getId());
}
@Override
......@@ -242,4 +245,12 @@ public class BrowserActionsTabModelSelector
public void addTabPersistentStoreObserver(TabPersistentStoreObserver observer) {
mTabSaver.addObserver(observer);
}
/**
* Remove a {@link TabPersistentStoreObserver} from {@link TabPersistentStore}.
* @param observer The observer to remove.
*/
public void removeTabPersistentStoreObserver(TabPersistentStoreObserver observer) {
mTabSaver.removeObserver(observer);
}
}
chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java
View file @ 1c3e620e
......@@ -424,4 +424,12 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
public void addTabPersistentStoreObserver(TabPersistentStoreObserver observer) {
mTabSaver.addObserver(observer);
}
/**
* Remove a {@link TabPersistentStoreObserver} from {@link TabPersistentStore}.
* @param observer The observer to remove.
*/
public void removeTabPersistentStoreObserver(TabPersistentStoreObserver observer) {
mTabSaver.removeObserver(observer);
}
}
chrome/android/javatests/src/org/chromium/chrome/browser/browseractions/BrowserActionActivityTest.java
View file @ 1c3e620e
......@@ -519,7 +519,8 @@ public class BrowserActionActivityTest {
@Override
public boolean isSatisfied() {
return activity2.areTabModelsInitialized()
&& activity2.getTabModelSelector().isTabStateInitialized();
&& activity2.getTabModelSelector().isTabStateInitialized()
&& activity2.getActivityTab() != null;
}
});
String cta2ActivityTabUrl = activity2.getActivityTab().getUrl();
......
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