Commit 46d75c12 authored by Joshua Pawlicki's avatar Joshua Pawlicki Committed by Commit Bot

Revert "[Android] Remove accounts and sign-in information from Omaha requests"

This reverts commit 4502ddc7.

Reason for revert: The Clank team has raised some concerns about removing this. Until there is consensus, it seems safer to stay with the status quo, sorry.

Original change's description:
> [Android] Remove accounts and sign-in information from Omaha requests
> 
> This CL removes the code that used to add _numaccounts,
> _numgoogleaccountsondevice and _numsignedin attributes to Omaha
> requests.
> 
> Bug: 926273
> Change-Id: I36edef48dc4d51cbed70ad1a7e14284cfa1c772f
> Reviewed-on: https://chromium-review.googlesource.com/c/1451925
> Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Jochen Eisinger <jochen@chromium.org>
> Auto-Submit: Boris Sazonov <bsazonov@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#629111}

TBR=waffles@chromium.org,jochen@chromium.org,bsazonov@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 926273
Change-Id: If778ea99524b6c199c25292f66f04a07d3271bea
Reviewed-on: https://chromium-review.googlesource.com/c/1457674Reviewed-by: default avatarJoshua Pawlicki <waffles@chromium.org>
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629854}
parent b7d963ad
...@@ -13,9 +13,14 @@ import android.util.Xml; ...@@ -13,9 +13,14 @@ import android.util.Xml;
import org.xmlpull.v1.XmlSerializer; import org.xmlpull.v1.XmlSerializer;
import org.chromium.base.BuildInfo; import org.chromium.base.BuildInfo;
import org.chromium.base.Log;
import org.chromium.base.ThreadUtils;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.browser.identity.SettingsSecureBasedIdentificationGenerator; import org.chromium.chrome.browser.identity.SettingsSecureBasedIdentificationGenerator;
import org.chromium.chrome.browser.identity.UniqueIdentificationGeneratorFactory; import org.chromium.chrome.browser.identity.UniqueIdentificationGeneratorFactory;
import org.chromium.chrome.browser.init.ProcessInitializationHandler;
import org.chromium.components.signin.AccountManagerFacade;
import org.chromium.components.signin.ChromeSigninController;
import org.chromium.ui.base.DeviceFormFactor; import org.chromium.ui.base.DeviceFormFactor;
import java.io.IOException; import java.io.IOException;
...@@ -98,6 +103,12 @@ public abstract class RequestGenerator { ...@@ -98,6 +103,12 @@ public abstract class RequestGenerator {
serializer.attribute(null, "lang", getLanguage()); serializer.attribute(null, "lang", getLanguage());
serializer.attribute(null, "installage", String.valueOf(installAge)); serializer.attribute(null, "installage", String.valueOf(installAge));
serializer.attribute(null, "ap", getAdditionalParameters()); serializer.attribute(null, "ap", getAdditionalParameters());
// <code>_numaccounts</code> is actually number of profiles, which is always one for
// Chrome Android.
serializer.attribute(null, "_numaccounts", "1");
serializer.attribute(null, "_numgoogleaccountsondevice",
String.valueOf(getNumGoogleAccountsOnDevice()));
serializer.attribute(null, "_numsignedin", String.valueOf(getNumSignedIn()));
serializer.attribute( serializer.attribute(
null, "_dl_mgr_disabled", String.valueOf(getDownloadManagerState())); null, "_dl_mgr_disabled", String.valueOf(getDownloadManagerState()));
...@@ -176,6 +187,43 @@ public abstract class RequestGenerator { ...@@ -176,6 +187,43 @@ public abstract class RequestGenerator {
return applicationLabel + ";" + brand + ";" + model; return applicationLabel + ";" + brand + ";" + model;
} }
/**
* Returns the number of accounts on the device, bucketed into:
* 0 accounts, 1 account, or 2+ accounts.
*
* @return Number of accounts on the device, bucketed as above.
*/
@VisibleForTesting
public int getNumGoogleAccountsOnDevice() {
// RequestGenerator may be invoked from JobService or AlarmManager (through OmahaService),
// so have to make sure AccountManagerFacade instance is initialized.
ThreadUtils.runOnUiThreadBlocking(
() -> ProcessInitializationHandler.getInstance().initializePreNative());
int numAccounts = 0;
try {
numAccounts = AccountManagerFacade.get().getGoogleAccounts().size();
} catch (Exception e) {
Log.e(TAG, "Can't get number of accounts.", e);
}
switch (numAccounts) {
case 0:
return 0;
case 1:
return 1;
default:
return 2;
}
}
/**
* Determine number of accounts signed in.
*/
@VisibleForTesting
public int getNumSignedIn() {
// We only have a single account.
return ChromeSigninController.get().isSignedIn() ? 1 : 0;
}
/** /**
* Returns DownloadManager system service enabled state as * Returns DownloadManager system service enabled state as
* -1 - manager state unknown * -1 - manager state unknown
......
...@@ -22,6 +22,7 @@ import org.chromium.base.test.util.Feature; ...@@ -22,6 +22,7 @@ import org.chromium.base.test.util.Feature;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.omaha.MockRequestGenerator; import org.chromium.chrome.test.omaha.MockRequestGenerator;
import org.chromium.chrome.test.omaha.MockRequestGenerator.DeviceType; import org.chromium.chrome.test.omaha.MockRequestGenerator.DeviceType;
import org.chromium.chrome.test.omaha.MockRequestGenerator.SignedInStatus;
import java.io.ByteArrayInputStream; import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
...@@ -87,8 +88,8 @@ public class OmahaBaseTest { ...@@ -87,8 +88,8 @@ public class OmahaBaseTest {
@Override @Override
protected RequestGenerator createRequestGenerator(Context context) { protected RequestGenerator createRequestGenerator(Context context) {
mMockGenerator = new MockRequestGenerator( mMockGenerator = new MockRequestGenerator(context,
context, mIsOnTablet ? DeviceType.TABLET : DeviceType.HANDSET); mIsOnTablet ? DeviceType.TABLET : DeviceType.HANDSET, SignedInStatus.FALSE);
return mMockGenerator; return mMockGenerator;
} }
......
...@@ -22,6 +22,7 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner; ...@@ -22,6 +22,7 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.omaha.AttributeFinder; import org.chromium.chrome.test.omaha.AttributeFinder;
import org.chromium.chrome.test.omaha.MockRequestGenerator; import org.chromium.chrome.test.omaha.MockRequestGenerator;
import org.chromium.chrome.test.omaha.MockRequestGenerator.DeviceType; import org.chromium.chrome.test.omaha.MockRequestGenerator.DeviceType;
import org.chromium.chrome.test.omaha.MockRequestGenerator.SignedInStatus;
import org.chromium.components.signin.AccountManagerFacade; import org.chromium.components.signin.AccountManagerFacade;
import org.chromium.components.signin.test.util.AccountHolder; import org.chromium.components.signin.test.util.AccountHolder;
import org.chromium.components.signin.test.util.FakeAccountManagerDelegate; import org.chromium.components.signin.test.util.FakeAccountManagerDelegate;
...@@ -76,7 +77,7 @@ public class RequestGeneratorTest { ...@@ -76,7 +77,7 @@ public class RequestGeneratorTest {
UniqueIdentificationGeneratorFactory.clearGeneratorMapForTest(); UniqueIdentificationGeneratorFactory.clearGeneratorMapForTest();
// Creating a RequestGenerator should register the identification generator. // Creating a RequestGenerator should register the identification generator.
new MockRequestGenerator(context, DeviceType.HANDSET); new MockRequestGenerator(context, DeviceType.HANDSET, SignedInStatus.FALSE);
// Verify the identification generator exists and is of the correct type. // Verify the identification generator exists and is of the correct type.
UniqueIdentificationGenerator instance = UniqueIdentificationGeneratorFactory.getInstance( UniqueIdentificationGenerator instance = UniqueIdentificationGeneratorFactory.getInstance(
...@@ -88,49 +89,88 @@ public class RequestGeneratorTest { ...@@ -88,49 +89,88 @@ public class RequestGeneratorTest {
@SmallTest @SmallTest
@Feature({"Omaha"}) @Feature({"Omaha"})
public void testHandsetXMLCreationWithInstall() { public void testHandsetXMLCreationWithInstall() {
createAndCheckXML(DeviceType.HANDSET, true); createAndCheckXML(DeviceType.HANDSET, SignedInStatus.FALSE, true);
} }
@Test @Test
@SmallTest @SmallTest
@Feature({"Omaha"}) @Feature({"Omaha"})
public void testHandsetXMLCreationWithoutInstall() { public void testHandsetXMLCreationWithoutInstall() {
createAndCheckXML(DeviceType.HANDSET, false); createAndCheckXML(DeviceType.HANDSET, SignedInStatus.FALSE, false);
} }
@Test @Test
@SmallTest @SmallTest
@Feature({"Omaha"}) @Feature({"Omaha"})
public void testTabletXMLCreationWithInstall() { public void testTabletXMLCreationWithInstall() {
createAndCheckXML(DeviceType.TABLET, true); createAndCheckXML(DeviceType.TABLET, SignedInStatus.FALSE, true);
} }
@Test @Test
@SmallTest @SmallTest
@Feature({"Omaha"}) @Feature({"Omaha"})
public void testTabletXMLCreationWithoutInstall() { public void testTabletXMLCreationWithoutInstall() {
createAndCheckXML(DeviceType.TABLET, false); createAndCheckXML(DeviceType.TABLET, SignedInStatus.FALSE, false);
} }
@Test @Test
@SmallTest @SmallTest
@Feature({"Omaha"}) @Feature({"Omaha"})
public void testIsSignedIn() { public void testIsSignedIn() {
createAndCheckXML(DeviceType.HANDSET, false); createAndCheckXML(DeviceType.HANDSET, SignedInStatus.TRUE, false);
} }
@Test @Test
@SmallTest @SmallTest
@Feature({"Omaha"}) @Feature({"Omaha"})
public void testIsNotSignedIn() { public void testIsNotSignedIn() {
createAndCheckXML(DeviceType.HANDSET, false); createAndCheckXML(DeviceType.HANDSET, SignedInStatus.FALSE, false);
}
@Test
@SmallTest
@Feature({"Omaha"})
public void testNoGoogleAccountsRetrieved() {
RequestGenerator generator =
createAndCheckXML(DeviceType.HANDSET, SignedInStatus.TRUE, false);
Assert.assertEquals(0, generator.getNumGoogleAccountsOnDevice());
}
@Test
@SmallTest
@Feature({"Omaha"})
public void testOneGoogleAccountRetrieved() {
RequestGenerator generator = createAndCheckXML(DeviceType.HANDSET, SignedInStatus.TRUE,
false, new Account("clanktester@this.com", "com.google"));
Assert.assertEquals(1, generator.getNumGoogleAccountsOnDevice());
}
@Test
@SmallTest
@Feature({"Omaha"})
public void testTwoGoogleAccountsRetrieved() {
RequestGenerator generator = createAndCheckXML(DeviceType.HANDSET, SignedInStatus.TRUE,
false, new Account("clanktester@gmail.com", "com.google"),
new Account("googleguy@elsewhere.com", "com.google"));
Assert.assertEquals(2, generator.getNumGoogleAccountsOnDevice());
}
@Test
@SmallTest
@Feature({"Omaha"})
public void testThreeGoogleAccountsExist() {
RequestGenerator generator = createAndCheckXML(DeviceType.HANDSET, SignedInStatus.TRUE,
false, new Account("clanktester@gmail.com", "com.google"),
new Account("googleguy@elsewhere.com", "com.google"),
new Account("ImInATest@gmail.com", "com.google"));
Assert.assertEquals(2, generator.getNumGoogleAccountsOnDevice());
} }
/** /**
* Checks that the XML is being created properly. * Checks that the XML is being created properly.
*/ */
private RequestGenerator createAndCheckXML( private RequestGenerator createAndCheckXML(DeviceType deviceType, SignedInStatus signInStatus,
DeviceType deviceType, boolean sendInstallEvent, Account... accounts) { boolean sendInstallEvent, Account... accounts) {
Context targetContext = InstrumentationRegistry.getTargetContext(); Context targetContext = InstrumentationRegistry.getTargetContext();
AdvancedMockContext context = new AdvancedMockContext(targetContext); AdvancedMockContext context = new AdvancedMockContext(targetContext);
...@@ -146,7 +186,9 @@ public class RequestGeneratorTest { ...@@ -146,7 +186,9 @@ public class RequestGeneratorTest {
String version = "1.2.3.4"; String version = "1.2.3.4";
long installAge = 42; long installAge = 42;
MockRequestGenerator generator = new MockRequestGenerator(context, deviceType); MockRequestGenerator generator =
new MockRequestGenerator(context, deviceType, signInStatus);
String xml = null; String xml = null;
try { try {
RequestData data = new RequestData(sendInstallEvent, 0, requestId, INSTALL_SOURCE); RequestData data = new RequestData(sendInstallEvent, 0, requestId, INSTALL_SOURCE);
...@@ -184,6 +226,12 @@ public class RequestGeneratorTest { ...@@ -184,6 +226,12 @@ public class RequestGeneratorTest {
checkForAttributeAndValue(xml, "request", "userid", "{" + generator.getDeviceID() + "}"); checkForAttributeAndValue(xml, "request", "userid", "{" + generator.getDeviceID() + "}");
checkForAttributeAndValue(xml, "app", "_numaccounts", "1");
checkForAttributeAndValue(xml, "app", "_numgoogleaccountsondevice",
String.valueOf(generator.getNumGoogleAccountsOnDevice()));
checkForAttributeAndValue(
xml, "app", "_numsignedin", String.valueOf(generator.getNumSignedIn()));
return generator; return generator;
} }
......
...@@ -14,6 +14,8 @@ public class MockRequestGenerator extends RequestGenerator { ...@@ -14,6 +14,8 @@ public class MockRequestGenerator extends RequestGenerator {
HANDSET, TABLET HANDSET, TABLET
} }
public enum SignedInStatus { TRUE, FALSE }
public static final String UUID_PHONE = "uuid_phone"; public static final String UUID_PHONE = "uuid_phone";
public static final String UUID_TABLET = "uuid_tablet"; public static final String UUID_TABLET = "uuid_tablet";
public static final String SERVER_URL = "http://totallylegitserver.com"; public static final String SERVER_URL = "http://totallylegitserver.com";
...@@ -27,9 +29,13 @@ public class MockRequestGenerator extends RequestGenerator { ...@@ -27,9 +29,13 @@ public class MockRequestGenerator extends RequestGenerator {
private final boolean mIsOnTablet; private final boolean mIsOnTablet;
public MockRequestGenerator(Context context, DeviceType deviceType) { private final boolean mIsSignedIn;
public MockRequestGenerator(
Context context, DeviceType deviceType, SignedInStatus signInStatus) {
super(context); super(context);
mIsOnTablet = deviceType == DeviceType.TABLET; mIsOnTablet = deviceType == DeviceType.TABLET;
mIsSignedIn = signInStatus == SignedInStatus.TRUE;
} }
@Override @Override
...@@ -72,6 +78,11 @@ public class MockRequestGenerator extends RequestGenerator { ...@@ -72,6 +78,11 @@ public class MockRequestGenerator extends RequestGenerator {
return LANGUAGE; return LANGUAGE;
} }
@Override
public int getNumSignedIn() {
return mIsSignedIn ? 1 : 0;
}
@Override @Override
public String getAdditionalParameters() { public String getAdditionalParameters() {
return ADDITIONAL_PARAMETERS; return ADDITIONAL_PARAMETERS;
......
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