Skip to content

Commit

Permalink
Weblayer: remove Chrome dependencies from installedapp java code
Browse files Browse the repository at this point in the history 
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 <tedchoc@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#857496}
  • Loading branch information
Evan Stade authored and Chromium LUCI CQ committed Feb 25, 2021
1 parent 6f8fe3c commit 6e6206e
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 205 deletions.
31 changes: 7 additions & 24 deletions ...ndroid/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<InstalledAppProvider> {
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);
}
}
76 changes: 39 additions & 37 deletions ...e/android/java/src/org/chromium/chrome/browser/installedapp/InstalledAppProviderImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) -> {
Expand Down Expand Up @@ -191,7 +196,7 @@ private void onFilteredInstalledApps(ArrayList<RelatedApplication> 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.
Expand All @@ -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;
}
Expand Down Expand Up @@ -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));
Expand All @@ -265,10 +271,6 @@ private void checkWebApk(
});
}

protected Profile getProfile() {
return Profile.getLastUsedRegularProfile();
}

/**
* Sets the version information, if available, to |installedApp|.
* @param installedApp
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -509,7 +511,7 @@ protected void delayThenRun(Runnable r, long delayMillis) {

@NativeMethods
interface Natives {
void checkDigitalAssetLinksRelationshipForWebApk(
Profile profile, String webDomain, String manifestUrl, Callback<Boolean> callback);
void checkDigitalAssetLinksRelationshipForWebApk(BrowserContextHandle handle,
String webDomain, String manifestUrl, Callback<Boolean> callback);
}
}
81 changes: 30 additions & 51 deletions chrome/android/java/src/org/chromium/chrome/browser/installedapp/PackageHash.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<byte[]> 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<byte[]>();
}

@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");
Expand All @@ -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);
}
}
Loading

0 comments on commit 6e6206e

Please sign in to comment.