Commit e5becf35 authored by Tanya Gupta's avatar Tanya Gupta Committed by Commit Bot

Add additional requirements for displaying SendTab option on Android.

  Previously the only requirements for whether to display the option was that the feature
  was enabled and the user was not currently on an incognito web page.

  This CL adds the following requirements:
  - The sync type SEND_TAB_TO_SELF must be enabled.
  - The number of devices the user uses with sync must be >= 2.
  - The user must currently be on an HTTP or HTTPS page.
  - The user is not on an Android native page.
  - Active tab is not in inCognito mode

Bug: 921767
Change-Id: Ifab23067d34b97f5ef1026bf906a8b6fc656176f
Reviewed-on: https://chromium-review.googlesource.com/c/1399446
Commit-Queue: Tanya Gupta <tgupta@chromium.org>
Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626931}
parent 4096cd0a
...@@ -7,7 +7,10 @@ package org.chromium.chrome.browser.send_tab_to_self; ...@@ -7,7 +7,10 @@ package org.chromium.chrome.browser.send_tab_to_self;
import org.chromium.chrome.browser.ChromeActivity; import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.share.ShareActivity; import org.chromium.chrome.browser.share.ShareActivity;
import org.chromium.chrome.browser.sync.ProfileSyncService;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.util.UrlUtilities;
import org.chromium.components.sync.ModelType;
/** /**
* A simple activity that allows Chrome to expose send tab to self as an * A simple activity that allows Chrome to expose send tab to self as an
...@@ -20,10 +23,25 @@ public class SendTabToSelfShareActivity extends ShareActivity { ...@@ -20,10 +23,25 @@ public class SendTabToSelfShareActivity extends ShareActivity {
} }
public static boolean featureIsAvailable(Tab currentTab) { public static boolean featureIsAvailable(Tab currentTab) {
// TODO(tgupta): Add additional checks here to make sure the user // Check that sync requirements are met:
// has additional devices that they are signed into and syncing on. // User is syncing on at least 2 devices (including this one)
return ChromeFeatureList.isEnabled(ChromeFeatureList.SEND_TAB_TO_SELF) && // SendTabToSelf sync datatype is enabled
!currentTab.isIncognito(); ProfileSyncService syncService = ProfileSyncService.get();
boolean userEnabledSyncType =
syncService.getPreferredDataTypes().contains(ModelType.SEND_TAB_TO_SELF);
boolean syncRequirementsMet =
userEnabledSyncType && syncService.getNumberOfSyncedDevices() > 1;
// Check that the tab and web content requirements are met:
// The active tab is not in inCognito mode or on a native page
// User is viewing an HTTP or HTTPS page
boolean isHttpOrHttps = UrlUtilities.isHttpOrHttps(currentTab.getUrl());
boolean contentRequirementsMet =
isHttpOrHttps && !currentTab.isNativePage() && !currentTab.isIncognito();
// Return whether the feature is enabled and the criteria is met as defined above.
boolean featureEnabled = ChromeFeatureList.isEnabled(ChromeFeatureList.SEND_TAB_TO_SELF);
return featureEnabled && contentRequirementsMet && syncRequirementsMet;
} }
} }
...@@ -211,6 +211,10 @@ public class ProfileSyncService { ...@@ -211,6 +211,10 @@ public class ProfileSyncService {
return nativeGetSyncEnterCustomPassphraseBodyText(mNativeProfileSyncServiceAndroid); return nativeGetSyncEnterCustomPassphraseBodyText(mNativeProfileSyncServiceAndroid);
} }
public int getNumberOfSyncedDevices() {
return nativeGetNumberOfSyncedDevices(mNativeProfileSyncServiceAndroid);
}
/** /**
* Checks if sync is currently set to use a custom passphrase. The sync engine must be running * Checks if sync is currently set to use a custom passphrase. The sync engine must be running
* (isEngineInitialized() returns true) before calling this function. * (isEngineInitialized() returns true) before calling this function.
...@@ -642,6 +646,7 @@ public class ProfileSyncService { ...@@ -642,6 +646,7 @@ public class ProfileSyncService {
private native String nativeGetCurrentSignedInAccountText(long nativeProfileSyncServiceAndroid); private native String nativeGetCurrentSignedInAccountText(long nativeProfileSyncServiceAndroid);
private native String nativeGetSyncEnterCustomPassphraseBodyText( private native String nativeGetSyncEnterCustomPassphraseBodyText(
long nativeProfileSyncServiceAndroid); long nativeProfileSyncServiceAndroid);
private native int nativeGetNumberOfSyncedDevices(long nativeProfileSyncServiceAndroid);
private native int[] nativeGetActiveDataTypes(long nativeProfileSyncServiceAndroid); private native int[] nativeGetActiveDataTypes(long nativeProfileSyncServiceAndroid);
private native int[] nativeGetChosenDataTypes(long nativeProfileSyncServiceAndroid); private native int[] nativeGetChosenDataTypes(long nativeProfileSyncServiceAndroid);
private native int[] nativeGetPreferredDataTypes(long nativeProfileSyncServiceAndroid); private native int[] nativeGetPreferredDataTypes(long nativeProfileSyncServiceAndroid);
......
...@@ -2272,6 +2272,7 @@ chrome_test_java_sources = [ ...@@ -2272,6 +2272,7 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/search_engines/TemplateUrlServiceTestUtils.java", "javatests/src/org/chromium/chrome/browser/search_engines/TemplateUrlServiceTestUtils.java",
"javatests/src/org/chromium/chrome/browser/searchwidget/SearchActivityTest.java", "javatests/src/org/chromium/chrome/browser/searchwidget/SearchActivityTest.java",
"javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java", "javatests/src/org/chromium/chrome/browser/searchwidget/SearchWidgetProviderTest.java",
"javatests/src/org/chromium/chrome/browser/send_tab_to_self/SendTabToSelfShareActivityTest.java",
"javatests/src/org/chromium/chrome/browser/services/GoogleServicesManagerIntegrationTest.java", "javatests/src/org/chromium/chrome/browser/services/GoogleServicesManagerIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/shape_detection/ShapeDetectionTest.java", "javatests/src/org/chromium/chrome/browser/shape_detection/ShapeDetectionTest.java",
"javatests/src/org/chromium/chrome/browser/share/ShareMenuActionHandlerIntegrationTest.java", "javatests/src/org/chromium/chrome/browser/share/ShareMenuActionHandlerIntegrationTest.java",
......
// 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.send_tab_to_self;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.LargeTest;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Feature;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.sync.FakeProfileSyncService;
import org.chromium.chrome.browser.sync.ProfileSyncService;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.browser.TabLoadObserver;
import org.chromium.components.sync.ModelType;
import org.chromium.net.test.EmbeddedTestServer;
import java.util.HashSet;
/**
* Test suite for the SendTabToSelfShare Activity
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
"enable-features=" + ChromeFeatureList.SEND_TAB_TO_SELF})
public class SendTabToSelfShareActivityTest {
@Rule
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
private FakeProfileSyncService mProfileSyncService;
private EmbeddedTestServer mTestServer;
private Tab mTab;
private static final String TEST_PAGE = "/chrome/test/data/android/simple.html";
private static final String ABOUT_BLANK = "about:blank";
private static final String NATIVE_PAGE = "chrome://newtab";
@Before
public void setUp() throws Exception {
mActivityTestRule.startMainActivityOnBlankPage();
mTab = mActivityTestRule.getActivity().getActivityTab();
mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
ProfileSyncService.overrideForTests(new FakeProfileSyncService());
mProfileSyncService = (FakeProfileSyncService) ProfileSyncService.get();
}
});
}
@After
public void tearDown() throws Exception {
if (mTestServer != null) {
mTestServer.stopAndDestroyServer();
}
}
// Test enabling of share activity
@Test
@LargeTest
@Feature({"SendTabToSelf"})
public void testEnabled() throws Exception {
setChosenTypes(ModelType.SEND_TAB_TO_SELF);
mProfileSyncService.setNumberOfSyncedDevices(2);
loadUrlInTab(TEST_PAGE);
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
boolean result = SendTabToSelfShareActivity.featureIsAvailable(mTab);
Assert.assertTrue(
"SendTabToSelfShareActivity disabled because of unsupported type", result);
}
});
}
// Test disabling of share activity because sync model type is not supported.
@Test
@LargeTest
@Feature({"SendTabToSelf"})
public void testDisabledUnsupportedSyncModelType() throws Exception {
setChosenTypes(null);
mProfileSyncService.setNumberOfSyncedDevices(2);
loadUrlInTab(TEST_PAGE);
assertFeatureIsDisabled("SendTabToSelfShareActivity disabled because of unsupported type");
}
// Test disabling of share activity because <2 devices are being synced.
@Test
@LargeTest
@Feature({"Sync"})
public void testDisabledInsufficientSyncedDevices() throws Exception {
mProfileSyncService.setNumberOfSyncedDevices(0);
setChosenTypes(ModelType.SEND_TAB_TO_SELF);
loadUrlInTab(TEST_PAGE);
assertFeatureIsDisabled("SendTabToSelfShareActivity disabled because of insufficient "
+ "synced devices");
mProfileSyncService.setNumberOfSyncedDevices(1);
loadUrlInTab(TEST_PAGE);
assertFeatureIsDisabled("SendTabToSelfShareActivity disabled because of insufficient "
+ "synced devices");
}
// Test disabling of share feature because of URL
@Test
@LargeTest
@Feature({"SendTabToSelf"})
public void testDisableWrongUrl() throws Exception {
setChosenTypes(ModelType.SEND_TAB_TO_SELF);
mProfileSyncService.setNumberOfSyncedDevices(2);
assertFeatureIsDisabled("SendTabToSelfShareActivity disabled because of unsupported url");
}
// Test enabling of share activity
@Test
@LargeTest
@Feature({"SendTabToSelf"})
@CommandLineFlags.Add("disable-features=" + ChromeFeatureList.SEND_TAB_TO_SELF)
public void testFeatureDisabled() throws Exception {
setChosenTypes(ModelType.SEND_TAB_TO_SELF);
mProfileSyncService.setNumberOfSyncedDevices(2);
loadUrlInTab(TEST_PAGE);
assertFeatureIsDisabled("SendTabToSelfShareActivity disabled because of unsupported type");
}
private void assertFeatureIsDisabled(final String errorMessage) {
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
boolean result = SendTabToSelfShareActivity.featureIsAvailable(mTab);
Assert.assertFalse(errorMessage, result);
}
});
}
private void setChosenTypes(Integer type) {
HashSet<Integer> chosenTypes = new HashSet<Integer>();
if (type != null) {
chosenTypes.add(type);
}
mProfileSyncService.setChosenDataTypes(false, chosenTypes);
}
private void loadUrlInTab(String url) throws Exception {
String testServerUrl = mTestServer.getURL(url);
new TabLoadObserver(mTab).fullyLoadUrl(testServerUrl);
}
}
\ No newline at end of file
...@@ -4,6 +4,9 @@ ...@@ -4,6 +4,9 @@
package org.chromium.chrome.browser.sync; package org.chromium.chrome.browser.sync;
import java.util.HashSet;
import java.util.Set;
/** /**
* Fake some ProfileSyncService methods for testing. * Fake some ProfileSyncService methods for testing.
* *
...@@ -11,6 +14,8 @@ package org.chromium.chrome.browser.sync; ...@@ -11,6 +14,8 @@ package org.chromium.chrome.browser.sync;
*/ */
public class FakeProfileSyncService extends ProfileSyncService { public class FakeProfileSyncService extends ProfileSyncService {
private boolean mEngineInitialized; private boolean mEngineInitialized;
private int mNumberOfSyncedDevices;
private Set<Integer> mChosenTypes = new HashSet<>();
public FakeProfileSyncService() { public FakeProfileSyncService() {
super(); super();
...@@ -29,4 +34,23 @@ public class FakeProfileSyncService extends ProfileSyncService { ...@@ -29,4 +34,23 @@ public class FakeProfileSyncService extends ProfileSyncService {
public boolean isUsingSecondaryPassphrase() { public boolean isUsingSecondaryPassphrase() {
return true; return true;
} }
@Override
public int getNumberOfSyncedDevices() {
return mNumberOfSyncedDevices;
}
public void setNumberOfSyncedDevices(int numDevices) {
mNumberOfSyncedDevices = numDevices;
}
@Override
public void setChosenDataTypes(boolean syncEverything, Set<Integer> enabledTypes) {
mChosenTypes = enabledTypes;
}
@Override
public Set<Integer> getPreferredDataTypes() {
return mChosenTypes;
}
} }
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/sync/device_info_sync_service_factory.h"
#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/browser/sync/session_sync_service_factory.h" #include "chrome/browser/sync/session_sync_service_factory.h"
#include "chrome/browser/sync/sync_ui_util.h" #include "chrome/browser/sync/sync_ui_util.h"
...@@ -31,6 +32,9 @@ ...@@ -31,6 +32,9 @@
#include "components/strings/grit/components_strings.h" #include "components/strings/grit/components_strings.h"
#include "components/sync/base/model_type.h" #include "components/sync/base/model_type.h"
#include "components/sync/base/pref_names.h" #include "components/sync/base/pref_names.h"
#include "components/sync/device_info/device_info.h"
#include "components/sync/device_info/device_info_sync_service.h"
#include "components/sync/device_info/device_info_tracker.h"
#include "components/sync/driver/about_sync_util.h" #include "components/sync/driver/about_sync_util.h"
#include "components/sync/driver/sync_service_utils.h" #include "components/sync/driver/sync_service_utils.h"
#include "components/sync/engine/net/network_resources.h" #include "components/sync/engine/net/network_resources.h"
...@@ -50,6 +54,7 @@ using base::android::JavaParamRef; ...@@ -50,6 +54,7 @@ using base::android::JavaParamRef;
using base::android::ScopedJavaLocalRef; using base::android::ScopedJavaLocalRef;
using browser_sync::ProfileSyncService; using browser_sync::ProfileSyncService;
using content::BrowserThread; using content::BrowserThread;
using syncer::DeviceInfo;
using unified_consent::UrlKeyedDataCollectionConsentHelper; using unified_consent::UrlKeyedDataCollectionConsentHelper;
namespace { namespace {
...@@ -461,6 +466,20 @@ ProfileSyncServiceAndroid::GetCurrentSignedInAccountText( ...@@ -461,6 +466,20 @@ ProfileSyncServiceAndroid::GetCurrentSignedInAccountText(
base::ASCIIToUTF16(sync_username))); base::ASCIIToUTF16(sync_username)));
} }
jint ProfileSyncServiceAndroid::GetNumberOfSyncedDevices(
JNIEnv* env,
const JavaParamRef<jobject>&) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
syncer::DeviceInfoSyncService* device_sync_service =
DeviceInfoSyncServiceFactory::GetForProfile(profile_);
if (!device_sync_service) {
return 0;
}
const std::vector<std::unique_ptr<DeviceInfo>>& all_devices =
device_sync_service->GetDeviceInfoTracker()->GetAllDeviceInfo();
return all_devices.size();
}
ScopedJavaLocalRef<jstring> ScopedJavaLocalRef<jstring>
ProfileSyncServiceAndroid::GetSyncEnterCustomPassphraseBodyText( ProfileSyncServiceAndroid::GetSyncEnterCustomPassphraseBodyText(
JNIEnv* env, JNIEnv* env,
......
...@@ -147,6 +147,10 @@ class ProfileSyncServiceAndroid : public syncer::SyncServiceObserver { ...@@ -147,6 +147,10 @@ class ProfileSyncServiceAndroid : public syncer::SyncServiceObserver {
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj); const base::android::JavaParamRef<jobject>& obj);
jint GetNumberOfSyncedDevices(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
// UI string getters. // UI string getters.
base::android::ScopedJavaLocalRef<jstring> base::android::ScopedJavaLocalRef<jstring>
......
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