From 6e6206eea109d9da76143c7b1af5ff92339b292d Mon Sep 17 00:00:00 2001 From: Evan Stade Date: Thu, 25 Feb 2021 02:10:40 +0000 Subject: [PATCH] Weblayer: remove Chrome dependencies from installedapp java code This consists of: * using BrowserContextHandle/BrowserContext in place of Profile * injecting Instant Apps logic at construction time, instead of accessing it directly from InstalledAppProviderImpl, since that code is not yet componentized * Removing FrameUrlDelegate and rewriting tests not to need it (to minimize the amount of production code that can't be componentized) * Reworking PackageHash so that it doesn't assume there's at most one regular profile and at most one incognito profile, since WebLayer supports the existence of multiple browser contexts. Bug: 1130989 Change-Id: I007bff47d0af4844b134c417c1fdba2b4e3187f7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2706400 Reviewed-by: Ted Choc Reviewed-by: Rayan Kanso Commit-Queue: Evan Stade Cr-Commit-Position: refs/heads/master@{#857496} --- .../InstalledAppProviderFactory.java | 31 ++---- .../InstalledAppProviderImpl.java | 76 ++++++++------- .../browser/installedapp/PackageHash.java | 81 ++++++---------- .../InstalledAppProviderTest.java | 97 ++++++------------- .../browser/installedapp/PackageHashTest.java | 46 +++++---- .../installedapp/installed_app_verifier.cc | 10 +- .../chrome_browsing_data_remover_delegate.cc | 5 +- weblayer/app/content_main_delegate_impl.cc | 2 - 8 files changed, 143 insertions(+), 205 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderFactory.java b/chrome/android/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderFactory.java index e7fc5c14d41ac2..65cfc39adb247b 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderFactory.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderFactory.java @@ -5,42 +5,25 @@ package org.chromium.chrome.browser.installedapp; import org.chromium.chrome.browser.instantapps.InstantAppsHandler; +import org.chromium.chrome.browser.profiles.Profile; import org.chromium.content_public.browser.RenderFrameHost; +import org.chromium.content_public.browser.WebContentsStatics; import org.chromium.installedapp.mojom.InstalledAppProvider; import org.chromium.services.service_manager.InterfaceFactory; -import org.chromium.url.GURL; /** Factory to create instances of the InstalledAppProvider Mojo service. */ public class InstalledAppProviderFactory implements InterfaceFactory { - private final FrameUrlDelegateImpl mFrameUrlDelegate; - - private static final class FrameUrlDelegateImpl - implements InstalledAppProviderImpl.FrameUrlDelegate { - private final RenderFrameHost mRenderFrameHost; - - public FrameUrlDelegateImpl(RenderFrameHost renderFrameHost) { - mRenderFrameHost = renderFrameHost; - } - - @Override - public GURL getUrl() { - GURL url = mRenderFrameHost.getLastCommittedURL(); - return (url != null) ? url : GURL.emptyGURL(); - } - - @Override - public boolean isIncognito() { - return mRenderFrameHost.isIncognito(); - } - } + private final RenderFrameHost mRenderFrameHost; public InstalledAppProviderFactory(RenderFrameHost renderFrameHost) { - mFrameUrlDelegate = new FrameUrlDelegateImpl(renderFrameHost); + mRenderFrameHost = renderFrameHost; } @Override public InstalledAppProvider createImpl() { return new InstalledAppProviderImpl( - mFrameUrlDelegate, new PackageManagerDelegate(), InstantAppsHandler.getInstance()); + Profile.fromWebContents(WebContentsStatics.fromRenderFrameHost(mRenderFrameHost)), + mRenderFrameHost, new PackageManagerDelegate(), + InstantAppsHandler.getInstance()::isInstantAppAvailable); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderImpl.java index ef19556b7b6a7f..432d62ca8a9f08 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderImpl.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderImpl.java @@ -26,9 +26,9 @@ import org.chromium.base.annotations.NativeMethods; import org.chromium.base.task.PostTask; import org.chromium.base.task.TaskTraits; -import org.chromium.chrome.browser.instantapps.InstantAppsHandler; -import org.chromium.chrome.browser.profiles.Profile; +import org.chromium.components.embedder_support.browser_context.BrowserContextHandle; import org.chromium.components.webapk.lib.client.WebApkValidator; +import org.chromium.content_public.browser.RenderFrameHost; import org.chromium.content_public.browser.UiThreadTaskTraits; import org.chromium.installedapp.mojom.InstalledAppProvider; import org.chromium.installedapp.mojom.RelatedApplication; @@ -67,33 +67,37 @@ public class InstalledAppProviderImpl implements InstalledAppProvider { private static final String TAG = "InstalledAppProvider"; - private final FrameUrlDelegate mFrameUrlDelegate; - private final PackageManagerDelegate mPackageManagerDelegate; - private final InstantAppsHandler mInstantAppsHandler; - - /** - * Small interface for dynamically getting the URL of the current frame. - * - * Abstract to allow for testing. - */ - public static interface FrameUrlDelegate { + /** Used to inject Instant Apps logic into InstalledAppProviderImpl. */ + public interface InstantAppProvider { /** - * Gets the URL of the current frame. Can return null (if the frame has disappeared). + * Returns whether or not the instant app is available. + * + * @param url The URL where the instant app is located. + * @param checkHoldback Check if the app would be available if the user weren't in the + * holdback group. + * @param includeUserPrefersBrowser Function should return true if there's an instant app + * intent even if the user has opted out of instant apps. + * @return Whether or not the instant app specified by the entry in the page's manifest is + * either available, or would be available if the user wasn't in the holdback group. */ - public GURL getUrl(); - - /** - * Checks if we're in incognito. If the frame has disappeared this returns true. - */ - public boolean isIncognito(); + boolean isInstantAppAvailable( + String url, boolean checkHoldback, boolean includeUserPrefersBrowser); } - public InstalledAppProviderImpl(FrameUrlDelegate frameUrlDelegate, - PackageManagerDelegate packageManagerDelegate, InstantAppsHandler instantAppsHandler) { - assert instantAppsHandler != null; - mFrameUrlDelegate = frameUrlDelegate; + // May be null in tests. + private final BrowserContextHandle mBrowserContextHandle; + private final RenderFrameHost mRenderFrameHost; + private final PackageManagerDelegate mPackageManagerDelegate; + @Nullable + private final InstantAppProvider mInstantAppProvider; + + public InstalledAppProviderImpl(BrowserContextHandle browserContextHandle, + RenderFrameHost renderFrameHost, PackageManagerDelegate packageManagerDelegate, + @Nullable InstantAppProvider instantAppProvider) { + mBrowserContextHandle = browserContextHandle; + mRenderFrameHost = renderFrameHost; mPackageManagerDelegate = packageManagerDelegate; - mInstantAppsHandler = instantAppsHandler; + mInstantAppProvider = instantAppProvider; } @Override @@ -143,7 +147,8 @@ public void onResult(@Nullable RelatedApplication app, int taskIdx, int delayMs) @UiThread public void filterInstalledApps(final RelatedApplication[] relatedApps, final Url manifestUrl, final FilterInstalledAppsResponse callback) { - final GURL frameUrl = mFrameUrlDelegate.getUrl(); + GURL url = mRenderFrameHost.getLastCommittedURL(); + final GURL frameUrl = url == null ? GURL.emptyGURL() : url; int delayMillis = 0; int numTasks = Math.min(relatedApps.length, MAX_ALLOWED_RELATED_APPS); ResultHolder resultHolder = new ResultHolder(numTasks, (resultPair) -> { @@ -191,7 +196,7 @@ private void onFilteredInstalledApps(ArrayList installedApps Integer delayMs, FilterInstalledAppsResponse callback) { RelatedApplication[] installedAppsArray; - if (mFrameUrlDelegate.isIncognito()) { + if (mRenderFrameHost.isIncognito()) { // Don't expose the related apps if in incognito mode. This is done at // the last stage to prevent using this API as an incognito detector by // timing how long it takes the Promise to resolve. @@ -209,9 +214,10 @@ private void checkInstantApp( ResultHolder resultHolder, int taskIdx, RelatedApplication app, GURL frameUrl) { int delayMs = calculateDelayForPackageMs(app.id); - if (!mInstantAppsHandler.isInstantAppAvailable(frameUrl.getSpec(), - INSTANT_APP_HOLDBACK_ID_STRING.equals(app.id), - true /* includeUserPrefersBrowser */)) { + if (mInstantAppProvider != null + && !mInstantAppProvider.isInstantAppAvailable(frameUrl.getSpec(), + INSTANT_APP_HOLDBACK_ID_STRING.equals(app.id), + true /* includeUserPrefersBrowser */)) { delayThenRun(() -> resultHolder.onResult(null, taskIdx, delayMs), 0); return; } @@ -255,7 +261,7 @@ private void checkWebApk( int delayMs = calculateDelayForPackageMs(app.url); InstalledAppProviderImplJni.get().checkDigitalAssetLinksRelationshipForWebApk( - getProfile(), app.url, manifestUrl.url, (verified) -> { + mBrowserContextHandle, app.url, manifestUrl.url, (verified) -> { if (verified) { PostTask.postTask(TaskTraits.BEST_EFFORT_MAY_BLOCK, () -> checkWebApkInstalled(resultHolder, taskIdx, app)); @@ -265,10 +271,6 @@ private void checkWebApk( }); } - protected Profile getProfile() { - return Profile.getLastUsedRegularProfile(); - } - /** * Sets the version information, if available, to |installedApp|. * @param installedApp @@ -334,7 +336,7 @@ private int calculateDelayForPackageMs(String packageName) { // add significant noise to the time taken to check whether this app is installed and // related. Otherwise, it would be possible to tell whether a non-related app is installed, // based on the time this operation takes. - short hash = PackageHash.hashForPackage(packageName, mFrameUrlDelegate.isIncognito()); + short hash = PackageHash.hashForPackage(packageName, mBrowserContextHandle); // The time delay is the low 10 bits of the hash in 100ths of a ms (between 0 and 10ms). int delayHundredthsOfMs = hash & 0x3ff; @@ -509,7 +511,7 @@ protected void delayThenRun(Runnable r, long delayMillis) { @NativeMethods interface Natives { - void checkDigitalAssetLinksRelationshipForWebApk( - Profile profile, String webDomain, String manifestUrl, Callback callback); + void checkDigitalAssetLinksRelationshipForWebApk(BrowserContextHandle handle, + String webDomain, String manifestUrl, Callback callback); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/installedapp/PackageHash.java b/chrome/android/java/src/org/chromium/chrome/browser/installedapp/PackageHash.java index 14a26a2702e387..6f91413c4bd530 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/installedapp/PackageHash.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/installedapp/PackageHash.java @@ -4,13 +4,14 @@ package org.chromium.chrome.browser.installedapp; +import android.util.SparseArray; + import androidx.annotation.VisibleForTesting; import org.chromium.base.ApiCompatibilityUtils; import org.chromium.base.annotations.CalledByNative; import org.chromium.chrome.browser.crypto.ByteArrayGenerator; -import org.chromium.chrome.browser.profiles.Profile; -import org.chromium.chrome.browser.profiles.ProfileManager; +import org.chromium.components.embedder_support.browser_context.BrowserContextHandle; import java.io.IOException; import java.security.GeneralSecurityException; @@ -40,63 +41,43 @@ * cleared. */ class PackageHash { - static class Salt implements ProfileManager.Observer { - private static Salt sInstance; - - private byte[] mProfileSalt; - private byte[] mIncognitoSalt; + // This map stores salts that have been calculated for different browser sessions (i.e. Browser + // Contexts). A SparseArray is used instead of a HashMap to avoid holding a reference to the key + // object. + private static SparseArray sSaltMap; - private Salt() { - reset(false); - ProfileManager.addObserver(this); - } - - void reset(boolean onlyIncognito) { - try { - if (!onlyIncognito) { - mProfileSalt = new ByteArrayGenerator().getBytes(20); - } - mIncognitoSalt = new ByteArrayGenerator().getBytes(20); - } catch (IOException | GeneralSecurityException e) { - // If this happens, the crypto source is messed up and we want the browser to crash. - throw new RuntimeException(e); - } - } + private static byte[] sGlobalSaltForTesting; - public static Salt getInstance() { - if (sInstance == null) { - sInstance = new Salt(); - } - return sInstance; - } + @VisibleForTesting + static byte[] getSaltBytes(BrowserContextHandle browserContext) { + if (sGlobalSaltForTesting != null) return sGlobalSaltForTesting; - public byte[] getSaltBytes(boolean isIncognito) { - return isIncognito ? mIncognitoSalt : mProfileSalt; + if (sSaltMap == null) { + sSaltMap = new SparseArray(); } - @Override - public void onProfileAdded(Profile profile) {} + byte[] salt = sSaltMap.get(browserContext.hashCode()); + if (salt != null) return salt; - @Override - public void onProfileDestroyed(Profile profile) { - if (profile.isOffTheRecord()) { - reset(true); - } + try { + salt = new ByteArrayGenerator().getBytes(20); + } catch (IOException | GeneralSecurityException e) { + // If this happens, the crypto source is messed up and we want the browser to crash. + throw new RuntimeException(e); } + sSaltMap.put(browserContext.hashCode(), salt); + return salt; + } - @VisibleForTesting - public static void setGlobalSaltForTesting(byte[] salt) { - if (sInstance == null) getInstance(); - sInstance.mProfileSalt = salt.clone(); - sInstance.mIncognitoSalt = salt.clone(); - } + static void setGlobalSaltForTesting(byte[] salt) { + sGlobalSaltForTesting = salt; } /** * Returns a SHA-256 hash of the package name, truncated to a 16-bit integer. */ - public static short hashForPackage(String packageName, boolean isIncognito) { - byte[] salt = Salt.getInstance().getSaltBytes(isIncognito); + static short hashForPackage(String packageName, BrowserContextHandle browserContext) { + byte[] salt = getSaltBytes(browserContext); Mac hasher; try { hasher = Mac.getInstance("HmacSHA256"); @@ -121,11 +102,9 @@ public static short hashForPackage(String packageName, boolean isIncognito) { } @CalledByNative - public static void onCookiesDeleted() { - if (Salt.sInstance == null) { - // Singleton is uninitialized. No need to reset the salt. - return; + public static void onCookiesDeleted(BrowserContextHandle browserContext) { + if (sSaltMap != null) { + sSaltMap.delete(browserContext.hashCode()); } - Salt.getInstance().reset(false); } } diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderTest.java index 52ba6c4f3b10d0..b19bf4308d3612 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderTest.java @@ -19,6 +19,9 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; import org.chromium.base.Callback; import org.chromium.base.test.BaseJUnit4ClassRunner; @@ -27,9 +30,10 @@ import org.chromium.base.test.util.CriteriaHelper; import org.chromium.base.test.util.DisabledTest; import org.chromium.base.test.util.JniMocker; -import org.chromium.chrome.browser.instantapps.InstantAppsHandler; -import org.chromium.chrome.browser.profiles.Profile; +import org.chromium.components.embedder_support.browser_context.BrowserContextHandle; +import org.chromium.content_public.browser.RenderFrameHost; import org.chromium.content_public.browser.test.NativeLibraryTestUtils; +import org.chromium.content_public.browser.test.mock.MockRenderFrameHost; import org.chromium.installedapp.mojom.InstalledAppProvider; import org.chromium.installedapp.mojom.RelatedApplication; import org.chromium.url.GURL; @@ -82,8 +86,9 @@ public class InstalledAppProviderTest { @Rule public JniMocker mocker = new JniMocker(); + @Mock + private MockRenderFrameHost mMockRenderFrameHost; private FakePackageManager mFakePackageManager; - private FakeFrameUrlDelegate mFrameUrlDelegate; private InstalledAppProviderTestImpl mInstalledAppProvider; private FakeInstantAppsHandler mFakeInstantAppsHandler; private TestInstalledAppProviderImplJni mTestInstalledAppProviderImplJni; @@ -135,10 +140,15 @@ public PackageInfo getPackageInfo(String packageName, int flags) private class InstalledAppProviderTestImpl extends InstalledAppProviderImpl { private long mLastDelayMillis; - public InstalledAppProviderTestImpl(FrameUrlDelegate frameUrlDelegate, + public InstalledAppProviderTestImpl(RenderFrameHost renderFrameHost, PackageManagerDelegate packageManagerDelegate, FakeInstantAppsHandler instantAppsHandler) { - super(frameUrlDelegate, packageManagerDelegate, instantAppsHandler); + super(new BrowserContextHandle() { + @Override + public long getNativeBrowserContextPointer() { + return 1; + } + }, renderFrameHost, packageManagerDelegate, instantAppsHandler::isInstantAppAvailable); } public long getLastDelayMillis() { @@ -155,11 +165,6 @@ protected void delayThenRun(Runnable r, long delayMillis) { public boolean isWebApkInstalled(String manifestUrl) { return mFakePackageManager.isWebApkInstalled(manifestUrl); } - - @Override - protected Profile getProfile() { - return null; - } } private static class TestInstalledAppProviderImplJni @@ -172,7 +177,8 @@ public void addVerfication(String webDomain, String manifestUrl) { @Override public void checkDigitalAssetLinksRelationshipForWebApk( - Profile profile, String webDomain, String manifestUrl, Callback callback) { + BrowserContextHandle browserContextHandle, String webDomain, String manifestUrl, + Callback callback) { callback.onResult(mRelationMap.containsKey(webDomain) && mRelationMap.get(webDomain).equals(manifestUrl)); } @@ -182,7 +188,7 @@ public void checkDigitalAssetLinksRelationshipForWebApk( * FakeInstantAppsHandler lets us mock getting RelatedApplications from a URL in the absence of * proper GMSCore calls. */ - private static class FakeInstantAppsHandler extends InstantAppsHandler { + private static class FakeInstantAppsHandler { private final List> mRelatedApplicationList; public FakeInstantAppsHandler() { @@ -200,7 +206,6 @@ public void resetForTest() { // TODO(thildebr): When the implementation of isInstantAppAvailable is complete, we need to // test its functionality instead of stubbing it out here. Instead we can create a wrapper // around the GMSCore functionality we need and override that here instead. - @Override public boolean isInstantAppAvailable( String url, boolean checkHoldback, boolean includeUserPrefersBrowser) { for (Pair pair : mRelatedApplicationList) { @@ -280,39 +285,6 @@ public String getString(int id) { } } - private static final class FakeFrameUrlDelegate - implements InstalledAppProviderImpl.FrameUrlDelegate { - private GURL mFrameUrl; - private boolean mIncognito; - - public FakeFrameUrlDelegate(String frameUrl) { - setFrameUrl(frameUrl); - } - - public void setFrameUrl(String frameUrl) { - if (frameUrl == null) { - mFrameUrl = GURL.emptyGURL(); - return; - } - - mFrameUrl = new GURL(frameUrl); - } - - @Override - public GURL getUrl() { - return mFrameUrl; - } - - public void setIncognito(boolean incognito) { - mIncognito = incognito; - } - - @Override - public boolean isIncognito() { - return mIncognito; - } - } - /** Creates a metaData bundle with a single resource-id key. */ private static Bundle createMetaData(String metaDataName, int metaDataResourceId) { Bundle metaData = new Bundle(); @@ -391,16 +363,18 @@ public void call(RelatedApplication[] installedRelatedApps) { @Before public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); NativeLibraryTestUtils.loadNativeLibraryNoBrowserProcess(); mTestInstalledAppProviderImplJni = new TestInstalledAppProviderImplJni(); mocker.mock(InstalledAppProviderImplJni.TEST_HOOKS, mTestInstalledAppProviderImplJni); mFakePackageManager = new FakePackageManager(); - mFrameUrlDelegate = new FakeFrameUrlDelegate(URL_ON_ORIGIN); + Mockito.when(mMockRenderFrameHost.getLastCommittedURL()) + .thenReturn(new GURL(URL_ON_ORIGIN)); mFakeInstantAppsHandler = new FakeInstantAppsHandler(); mInstalledAppProvider = new InstalledAppProviderTestImpl( - mFrameUrlDelegate, mFakePackageManager, mFakeInstantAppsHandler); + mMockRenderFrameHost, mFakePackageManager, mFakeInstantAppsHandler); } /** Origin of the page using the API is missing certain parts of the URI. */ @@ -413,24 +387,12 @@ public void testOriginMissingParts() throws Exception { setAssetStatement(PACKAGE_NAME_1, NAMESPACE_WEB, RELATION_HANDLE_ALL_URLS, ORIGIN); RelatedApplication[] expectedInstalledRelatedApps = new RelatedApplication[] {}; - mFrameUrlDelegate.setFrameUrl(ORIGIN_MISSING_SCHEME); - verifyInstalledApps(manifestRelatedApps, expectedInstalledRelatedApps); - - mFrameUrlDelegate.setFrameUrl(ORIGIN_MISSING_HOST); + Mockito.when(mMockRenderFrameHost.getLastCommittedURL()) + .thenReturn(new GURL(ORIGIN_MISSING_SCHEME)); verifyInstalledApps(manifestRelatedApps, expectedInstalledRelatedApps); - } - - /** Incognito mode with one related Android app. */ - @Test - @SmallTest - @UiThreadTest - public void testIncognitoWithOneInstalledRelatedApp() throws Exception { - RelatedApplication manifestRelatedApps[] = new RelatedApplication[] { - createRelatedApplication(PLATFORM_ANDROID, PACKAGE_NAME_1, null)}; - setAssetStatement(PACKAGE_NAME_1, NAMESPACE_WEB, RELATION_HANDLE_ALL_URLS, ORIGIN); - RelatedApplication[] expectedInstalledRelatedApps = new RelatedApplication[] {}; - mFrameUrlDelegate.setIncognito(true); + Mockito.when(mMockRenderFrameHost.getLastCommittedURL()) + .thenReturn(new GURL(ORIGIN_MISSING_HOST)); verifyInstalledApps(manifestRelatedApps, expectedInstalledRelatedApps); } @@ -620,14 +582,15 @@ public void testDynamicallyChangingUrl() throws Exception { verifyInstalledApps(manifestRelatedApps, expectedInstalledRelatedApps); // Simulate a navigation to a different origin. - mFrameUrlDelegate.setFrameUrl(ORIGIN_DIFFERENT_SCHEME); + Mockito.when(mMockRenderFrameHost.getLastCommittedURL()) + .thenReturn(new GURL(ORIGIN_DIFFERENT_SCHEME)); // Now the result should include the Android app that relates to the new origin. expectedInstalledRelatedApps = manifestRelatedApps; verifyInstalledApps(manifestRelatedApps, expectedInstalledRelatedApps); // Simulate the native RenderFrameHost disappearing. - mFrameUrlDelegate.setFrameUrl(null); + Mockito.when(mMockRenderFrameHost.getLastCommittedURL()).thenReturn(null); expectedInstalledRelatedApps = new RelatedApplication[] {}; verifyInstalledApps(manifestRelatedApps, expectedInstalledRelatedApps); @@ -929,7 +892,7 @@ public void testMultipleInstalledRelatedApps() throws Exception { public void testArtificialDelay() throws Exception { byte[] salt = {0x64, 0x09, -0x68, -0x25, 0x70, 0x11, 0x25, 0x24, 0x68, -0x1a, 0x08, 0x79, -0x12, -0x50, 0x3b, -0x57, -0x17, -0x4d, 0x46, 0x02}; - PackageHash.Salt.setGlobalSaltForTesting(salt); + PackageHash.setGlobalSaltForTesting(salt); setAssetStatement(PACKAGE_NAME_1, NAMESPACE_WEB, RELATION_HANDLE_ALL_URLS, ORIGIN); // Installed app. diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/installedapp/PackageHashTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/installedapp/PackageHashTest.java index 77c5ff616e8ac3..b8552fac019c55 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/installedapp/PackageHashTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/installedapp/PackageHashTest.java @@ -5,24 +5,25 @@ package org.chromium.chrome.browser.installedapp; import org.junit.Assert; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.junit.runners.BlockJUnit4ClassRunner; +import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.base.test.util.Feature; +import org.chromium.components.embedder_support.browser_context.BrowserContextHandle; import java.util.Arrays; /** * Tests that PackageHash generates correct hashes. */ -@RunWith(BlockJUnit4ClassRunner.class) +@RunWith(BaseRobolectricTestRunner.class) public class PackageHashTest { - @Before - public void setUp() { - // Initialize the Singleton. - PackageHash.Salt.getInstance(); + private static class FakeBrowserContextHandle implements BrowserContextHandle { + @Override + public long getNativeBrowserContextPointer() { + return 1; + } } @Test @@ -30,31 +31,40 @@ public void setUp() { public void testPackageHash() { byte[] salt = {0x64, 0x09, -0x68, -0x25, 0x70, 0x11, 0x25, 0x24, 0x68, -0x1a, 0x08, 0x79, -0x12, -0x50, 0x3b, -0x57, -0x17, -0x4d, 0x46, 0x02}; - PackageHash.Salt.setGlobalSaltForTesting(salt); + PackageHash.setGlobalSaltForTesting(salt); // These expectations are based on the salt + ':' + packageName, encoded in UTF-8, hashed // with SHA-256, and looking at the first two bytes of the result. - Assert.assertEquals((short) 0x0d4d, PackageHash.hashForPackage("com.example.test1", false)); + Assert.assertEquals((short) 0x0d4d, PackageHash.hashForPackage("com.example.test1", null)); Assert.assertEquals( - (short) 0xfa6f, PackageHash.hashForPackage("com.example.t\u00e9st2", false)); + (short) 0xfa6f, PackageHash.hashForPackage("com.example.t\u00e9st2", null)); byte[] salt2 = {-0x10, 0x38, -0x28, 0x1f, 0x59, 0x2d, -0x2d, -0x4a, 0x23, 0x76, 0x6d, -0x54, 0x27, -0x2d, -0x3f, -0x59, -0x2e, -0x0e, 0x67, 0x7a}; - PackageHash.Salt.setGlobalSaltForTesting(salt2); - Assert.assertEquals((short) 0xd6d6, PackageHash.hashForPackage("com.example.test1", false)); + PackageHash.setGlobalSaltForTesting(salt2); + Assert.assertEquals((short) 0xd6d6, PackageHash.hashForPackage("com.example.test1", null)); Assert.assertEquals( - (short) 0x5193, PackageHash.hashForPackage("com.example.t\u00e9st2", false)); + (short) 0x5193, PackageHash.hashForPackage("com.example.t\u00e9st2", null)); } @Test @Feature({"InstalledApp"}) - public void testSaltResets() { - PackageHash.Salt salt = PackageHash.Salt.getInstance(); - byte[] firstSalt = salt.getSaltBytes(/* isIncognito= */ true); + public void testBrowsingSessionSalts() { + BrowserContextHandle firstHandle = new FakeBrowserContextHandle(); + byte[] firstSalt = PackageHash.getSaltBytes(firstHandle); + // Verify the same salt is given a second time. + Assert.assertArrayEquals(firstSalt, PackageHash.getSaltBytes(firstHandle)); - PackageHash.onCookiesDeleted(); - byte[] postDeletionSalt = salt.getSaltBytes(/* isIncognito= */ true); + // Verify a different salt is returned for a different browsing session. + BrowserContextHandle secondHandle = new FakeBrowserContextHandle(); + byte[] secondSalt = PackageHash.getSaltBytes(secondHandle); + Assert.assertFalse(Arrays.equals(firstSalt, secondSalt)); // The salt should have been updated after cookies were deleted. + PackageHash.onCookiesDeleted(firstHandle); + byte[] postDeletionSalt = PackageHash.getSaltBytes(firstHandle); Assert.assertFalse(Arrays.equals(firstSalt, postDeletionSalt)); + + // But not for the other browsing session. + Assert.assertArrayEquals(secondSalt, PackageHash.getSaltBytes(secondHandle)); } } diff --git a/chrome/browser/android/installedapp/installed_app_verifier.cc b/chrome/browser/android/installedapp/installed_app_verifier.cc index bf6e9788b618ff..9939313cfc2041 100644 --- a/chrome/browser/android/installedapp/installed_app_verifier.cc +++ b/chrome/browser/android/installedapp/installed_app_verifier.cc @@ -10,8 +10,7 @@ #include "base/android/jni_string.h" #include "chrome/android/chrome_jni_headers/InstalledAppProviderImpl_jni.h" #include "chrome/browser/installable/digital_asset_links/digital_asset_links_handler.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/profiles/profile_android.h" +#include "components/embedder_support/android/browser_context/browser_context_handle.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/storage_partition.h" @@ -29,11 +28,12 @@ void DidGetResult( void JNI_InstalledAppProviderImpl_CheckDigitalAssetLinksRelationshipForWebApk( JNIEnv* env, - const base::android::JavaParamRef& jprofile, + const base::android::JavaParamRef& jhandle, const base::android::JavaParamRef& jwebDomain, const base::android::JavaParamRef& jmanifestUrl, const base::android::JavaParamRef& jcallback) { - Profile* profile = ProfileAndroid::FromProfileAndroid(jprofile); + content::BrowserContext* browser_context = + browser_context::BrowserContextFromJavaHandle(jhandle); std::string web_domain = ConvertJavaStringToUTF8(env, jwebDomain); std::string manifest_url = ConvertJavaStringToUTF8(env, jmanifestUrl); @@ -43,7 +43,7 @@ void JNI_InstalledAppProviderImpl_CheckDigitalAssetLinksRelationshipForWebApk( auto handler = std::make_unique( - content::BrowserContext::GetDefaultStoragePartition(profile) + content::BrowserContext::GetDefaultStoragePartition(browser_context) ->GetURLLoaderFactoryForBrowserProcess()); auto* handler_ptr = handler.get(); diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc index 60a5a62a3812ff..59a8da3a96f31a 100644 --- a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc +++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc @@ -143,6 +143,7 @@ #include "chrome/browser/android/search_permissions/search_permissions_service.h" #include "chrome/browser/android/webapps/webapp_registry.h" #include "chrome/browser/offline_pages/offline_page_model_factory.h" +#include "chrome/browser/profiles/profile_android.h" #include "components/cdm/browser/media_drm_storage_impl.h" // nogncheck crbug.com/1125897 #include "components/feed/buildflags.h" #include "components/feed/core/v2/public/feed_service.h" @@ -689,7 +690,9 @@ void ChromeBrowsingDataRemoverDelegate::RemoveEmbedderData( MediaDeviceIDSalt::Reset(profile_->GetPrefs()); #if defined(OS_ANDROID) - Java_PackageHash_onCookiesDeleted(base::android::AttachCurrentThread()); + Java_PackageHash_onCookiesDeleted( + base::android::AttachCurrentThread(), + ProfileAndroid::FromProfile(profile_)->GetJavaObject()); #endif } diff --git a/weblayer/app/content_main_delegate_impl.cc b/weblayer/app/content_main_delegate_impl.cc index ee64502d4b6bc1..d76dc8a2954142 100644 --- a/weblayer/app/content_main_delegate_impl.cc +++ b/weblayer/app/content_main_delegate_impl.cc @@ -160,8 +160,6 @@ bool ContentMainDelegateImpl::BasicStartupComplete(int* exit_code) { #endif // TODO(crbug.com/1057770): make Background Fetch work with WebLayer. ::features::kBackgroundFetch, - // TODO(crbug.com/1130989): Support GetInstalledRelatedApps on WebLayer. - ::features::kInstalledApp, // TODO(crbug.com/1091212): make Notification triggers work with // WebLayer. ::features::kNotificationTriggers,