Skip to content

Commit

Permalink
[3/3] Rename SafeBrowsingApiHandler to SafetyNetApiHandler
Browse files Browse the repository at this point in the history
We will migrate to a new Safe Browsing API that is built in a
dedicated SafeBrowsing module, rename the current handler to
SafetyNetApiHandler.

This is a three-sided patch:
1) Land new class name with additional compatibility shims.
2) Rename in downstream.
3) (this CL) Rename in upstream and remove the compatibility shims.

Bug: 1444515
Change-Id: Ia9afc7e99cea474a038050bd722b5a1460c78800
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4528489
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: thefrog <thefrog@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146769}
  • Loading branch information
Xinghui Lu authored and Chromium LUCI CQ committed May 19, 2023
1 parent 2188d72 commit a63ea2f
Show file tree
Hide file tree
Showing 14 changed files with 107 additions and 123 deletions.
Expand Up @@ -55,7 +55,7 @@
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.InMemorySharedPreferences;
import org.chromium.components.safe_browsing.SafeBrowsingApiBridge;
import org.chromium.components.safe_browsing.SafeBrowsingApiHandler;
import org.chromium.components.safe_browsing.SafetyNetApiHandler;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.content_public.common.ContentUrlConstants;
import org.chromium.net.test.EmbeddedTestServer;
Expand Down Expand Up @@ -131,10 +131,10 @@ public AwBrowserContext createAwBrowserContextOnUiThread(InMemorySharedPreferenc
private static final String WEB_UI_HOST = "safe-browsing";

/**
* A fake SafeBrowsingApiHandler which treats URLs ending in MALWARE_HTML_PATH as malicious URLs
* A fake SafetyNetApiHandler which treats URLs ending in MALWARE_HTML_PATH as malicious URLs
* that should be blocked.
*/
public static class MockSafeBrowsingApiHandler implements SafeBrowsingApiHandler {
public static class MockSafetyNetApiHandler implements SafetyNetApiHandler {
private Observer mObserver;
private static final String SAFE_METADATA = "{}";

Expand Down Expand Up @@ -192,12 +192,12 @@ public boolean startAllowlistLookup(final String uri, int threatType) {
}

/**
* A fake AwBrowserContext which loads the MockSafeBrowsingApiHandler instead of the real one.
* A fake AwBrowserContext which loads the MockSafetyNetApiHandler instead of the real one.
*/
private static class MockAwBrowserContext extends AwBrowserContext {
public MockAwBrowserContext(SharedPreferences sharedPreferences) {
super(sharedPreferences, 0, true);
SafeBrowsingApiBridge.setHandler(new MockSafeBrowsingApiHandler());
SafeBrowsingApiBridge.setHandler(new MockSafetyNetApiHandler());
}
}

Expand Down
2 changes: 1 addition & 1 deletion chrome/android/chrome_test_java_sources.gni
Expand Up @@ -26,7 +26,7 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/JavaScriptEvalChromeTest.java",
"javatests/src/org/chromium/chrome/browser/LauncherShortcutTest.java",
"javatests/src/org/chromium/chrome/browser/MainActivityWithURLTest.java",
"javatests/src/org/chromium/chrome/browser/MockSafeBrowsingApiHandler.java",
"javatests/src/org/chromium/chrome/browser/MockSafetyNetApiHandler.java",
"javatests/src/org/chromium/chrome/browser/NavigateTest.java",
"javatests/src/org/chromium/chrome/browser/OmahaServiceStartDelayerIntegrationTest.java",
"javatests/src/org/chromium/chrome/browser/PopularUrlsTest.java",
Expand Down
Expand Up @@ -10,21 +10,21 @@

import org.chromium.base.task.PostTask;
import org.chromium.base.task.TaskTraits;
import org.chromium.components.safe_browsing.SafeBrowsingApiHandler;
import org.chromium.components.safe_browsing.SafetyNetApiHandler;

import java.util.HashMap;
import java.util.Map;

/**
* SafeBrowsingApiHandler that vends fake responses.
* SafetyNetApiHandler that vends fake responses.
*/
public class MockSafeBrowsingApiHandler implements SafeBrowsingApiHandler {
public class MockSafetyNetApiHandler implements SafetyNetApiHandler {
private Observer mObserver;
// Mock time it takes for a lookup request to complete.
private static final long DEFAULT_CHECK_DELTA_US = 10;
private static final String SAFE_METADATA = "{}";

// Global url -> metadataResponse map. In practice there is only one SafeBrowsingApiHandler, but
// Global url -> metadataResponse map. In practice there is only one SafetyNetApiHandler, but
// it is cumbersome for tests to reach into the singleton instance directly. So just make this
// static and modifiable from java tests using a static method.
private static final Map<String, String> sResponseMap = new HashMap<>();
Expand Down
Expand Up @@ -64,7 +64,7 @@ private int getNumInfobarsShowing() {

@Before
public void setUp() throws Exception {
SafeBrowsingApiBridge.setHandler(new MockSafeBrowsingApiHandler());
SafeBrowsingApiBridge.setHandler(new MockSafetyNetApiHandler());
mActivityTestRule.startMainActivityOnBlankPage();

PostTask.runOrPostTask(
Expand All @@ -78,7 +78,7 @@ public void setUp() throws Exception {
@After
public void tearDown() {
mTestServer.stopAndDestroyServer();
MockSafeBrowsingApiHandler.clearMockResponses();
MockSafetyNetApiHandler.clearMockResponses();
}

@Test
Expand Down Expand Up @@ -114,7 +114,7 @@ public void testAbusiveGesturePopupBlocked() throws Exception {
final TabModelSelector selector = mActivityTestRule.getActivity().getTabModelSelector();

String url = mTestServer.getURL("/chrome/test/data/android/popup_on_click.html");
MockSafeBrowsingApiHandler.addMockResponse(url, METADATA_FOR_ABUSIVE_ENFORCEMENT);
MockSafetyNetApiHandler.addMockResponse(url, METADATA_FOR_ABUSIVE_ENFORCEMENT);

mActivityTestRule.loadUrl(url);
CriteriaHelper.pollUiThread(
Expand Down
Expand Up @@ -31,7 +31,7 @@
import org.chromium.ui.base.PageTransition;

/**
* Test integration with the SafeBrowsingApiHandler.
* Test integration with the SafetyNetApiHandler.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
Expand Down Expand Up @@ -79,15 +79,15 @@ private void loadUrlNonBlocking(String url) {

@Before
public void setUp() {
SafeBrowsingApiBridge.setHandler(new MockSafeBrowsingApiHandler());
SafeBrowsingApiBridge.setHandler(new MockSafetyNetApiHandler());
}

@After
public void tearDown() {
if (mTestServer != null) {
mTestServer.stopAndDestroyServer();
}
MockSafeBrowsingApiHandler.clearMockResponses();
MockSafetyNetApiHandler.clearMockResponses();
}

@Test
Expand All @@ -108,7 +108,7 @@ public void interstitialPage() throws Exception {
mTestServer = EmbeddedTestServer.createAndStartServer(
ApplicationProvider.getApplicationContext());
String url = mTestServer.getURL("/chrome/test/data/android/about.html");
MockSafeBrowsingApiHandler.addMockResponse(url, "{\"matches\":[{\"threat_type\":\"5\"}]}");
MockSafetyNetApiHandler.addMockResponse(url, "{\"matches\":[{\"threat_type\":\"5\"}]}");
mActivityTestRule.startMainActivityOnBlankPage();

loadUrlNonBlocking(url);
Expand Down
Expand Up @@ -33,7 +33,7 @@
import org.chromium.base.test.util.CriteriaHelper;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.HistogramWatcher;
import org.chromium.chrome.browser.MockSafeBrowsingApiHandler;
import org.chromium.chrome.browser.MockSafetyNetApiHandler;
import org.chromium.chrome.browser.browserservices.verification.ChromeOriginVerifier;
import org.chromium.chrome.browser.document.ChromeLauncherActivity;
import org.chromium.chrome.browser.firstrun.FirstRunStatus;
Expand Down Expand Up @@ -578,7 +578,7 @@ private void testCanSetCookie(boolean afterNative) throws Exception {

private void testSafeBrowsingMainResource(boolean afterNative, boolean splitCacheEnabled)
throws Exception {
SafeBrowsingApiBridge.setHandler(new MockSafeBrowsingApiHandler());
SafeBrowsingApiBridge.setHandler(new MockSafetyNetApiHandler());
CustomTabsSessionToken session = prepareSession();

String cacheable = "/cachetime";
Expand All @@ -587,7 +587,7 @@ private void testSafeBrowsingMainResource(boolean afterNative, boolean splitCach
Uri url = Uri.parse(mServer.getURL(cacheable));

try {
MockSafeBrowsingApiHandler.addMockResponse(
MockSafetyNetApiHandler.addMockResponse(
url.toString(), "{\"matches\":[{\"threat_type\":\"5\"}]}");

Intent intent = CustomTabsIntentTestUtils.createMinimalCustomTabIntent(
Expand Down Expand Up @@ -616,19 +616,19 @@ private void testSafeBrowsingMainResource(boolean afterNative, boolean splitCach
Assert.assertEquals(1, readFromSocketCallback.getCallCount());
}
} finally {
MockSafeBrowsingApiHandler.clearMockResponses();
MockSafetyNetApiHandler.clearMockResponses();
}
}

private void testSafeBrowsingSubresource(boolean afterNative) throws Exception {
SafeBrowsingApiBridge.setHandler(new MockSafeBrowsingApiHandler());
SafeBrowsingApiBridge.setHandler(new MockSafetyNetApiHandler());
CustomTabsSessionToken session = prepareSession();
String cacheable = "/cachetime";
waitForDetachedRequest(session, cacheable, afterNative);
Uri url = Uri.parse(mServer.getURL(cacheable));

try {
MockSafeBrowsingApiHandler.addMockResponse(
MockSafetyNetApiHandler.addMockResponse(
url.toString(), "{\"matches\":[{\"threat_type\":\"5\"}]}");

String pageUrl = mServer.getURL("/chrome/test/data/android/cacheable_subresource.html");
Expand All @@ -644,7 +644,7 @@ private void testSafeBrowsingSubresource(boolean afterNative) throws Exception {
// robust check.
CriteriaHelper.pollUiThread(() -> webContents.getTitle().equals("Security error"));
} finally {
MockSafeBrowsingApiHandler.clearMockResponses();
MockSafetyNetApiHandler.clearMockResponses();
}
}

Expand Down
Expand Up @@ -23,7 +23,7 @@
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.CriteriaHelper;
import org.chromium.base.test.util.Restriction;
import org.chromium.chrome.browser.MockSafeBrowsingApiHandler;
import org.chromium.chrome.browser.MockSafetyNetApiHandler;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.infobar.InfoBarContainer;
Expand Down Expand Up @@ -100,7 +100,7 @@ private void createAndPublishRulesetDisallowingSuffix(String suffix) {
@Before
public void setUp() throws Exception {
mTestServer = mTestServerRule.getServer();
SafeBrowsingApiBridge.setHandler(new MockSafeBrowsingApiHandler());
SafeBrowsingApiBridge.setHandler(new MockSafetyNetApiHandler());
mActivityTestRule.startMainActivityOnBlankPage();

// Disallow all jpgs.
Expand All @@ -109,7 +109,7 @@ public void setUp() throws Exception {

@After
public void tearDown() {
MockSafeBrowsingApiHandler.clearMockResponses();
MockSafetyNetApiHandler.clearMockResponses();
}

@Test
Expand Down Expand Up @@ -322,7 +322,7 @@ public void didAddTab(Tab tab, @TabLaunchType int type,

private boolean loadPageWithBlockableContentAndTestIfBlocked(String url, String metadata)
throws TimeoutException {
MockSafeBrowsingApiHandler.addMockResponse(url, metadata);
MockSafetyNetApiHandler.addMockResponse(url, metadata);
mActivityTestRule.loadUrl(url);
return Boolean.parseBoolean(mActivityTestRule.runJavaScriptCodeInCurrentTab("imgLoaded"));
}
Expand Down
Expand Up @@ -34,7 +34,7 @@
import org.chromium.base.test.util.MinAndroidSdkLevel;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.ChromeTabbedActivity2;
import org.chromium.chrome.browser.MockSafeBrowsingApiHandler;
import org.chromium.chrome.browser.MockSafetyNetApiHandler;
import org.chromium.chrome.browser.customtabs.CustomTabActivityTestRule;
import org.chromium.chrome.browser.customtabs.CustomTabsIntentTestUtils;
import org.chromium.chrome.browser.flags.ChromeSwitches;
Expand Down Expand Up @@ -330,8 +330,8 @@ public void testNavigationFromSuspendedTabToInterstitial() {
startLoadingUrl(mTab, mStartingUrl);
waitForSuspendedTabToShow(mTab, STARTING_FQDN);

SafeBrowsingApiBridge.setHandler(new MockSafeBrowsingApiHandler());
MockSafeBrowsingApiHandler.addMockResponse(
SafeBrowsingApiBridge.setHandler(new MockSafetyNetApiHandler());
MockSafetyNetApiHandler.addMockResponse(
mDifferentUrl, "{\"matches\":[{\"threat_type\":\"5\"}]}");
startLoadingUrl(mTab, mDifferentUrl);

Expand Down
1 change: 0 additions & 1 deletion components/safe_browsing/android/BUILD.gn
Expand Up @@ -17,7 +17,6 @@ android_library("safe_browsing_java") {
annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
sources = [
"java/src/org/chromium/components/safe_browsing/SafeBrowsingApiBridge.java",
"java/src/org/chromium/components/safe_browsing/SafeBrowsingApiHandler.java",
"java/src/org/chromium/components/safe_browsing/SafetyNetApiHandler.java",
]
}
Expand Down
Expand Up @@ -15,7 +15,7 @@
/**
* Helper for calling GMSCore Safe Browsing API from native code.
*
* The {@link #setHandler(SafeBrowsingApiHandler)} must be invoked first. After that
* The {@link #setHandler(SafetyNetApiHandler)} must be invoked first. After that
* {@link #startUriLookup(long, String, int[])} and {@link #startAllowlistLookup(String, int)} can
* be used to check the URLs. The handler would be initialized lazily on the first URL check.
*
Expand All @@ -34,7 +34,7 @@ public final class SafeBrowsingApiBridge {
private static boolean sHandlerInitCalled;

@GuardedBy("sLock")
private static SafeBrowsingApiHandler sHandler;
private static SafetyNetApiHandler sHandler;

@GuardedBy("sLock")
private static UrlCheckTimeObserver sUrlCheckTimeObserver;
Expand All @@ -44,11 +44,11 @@ private SafeBrowsingApiBridge() {
}

/**
* Sets the {@link SafeBrowsingApiHandler} object once and for the lifetime of this process.
* Sets the {@link SafetyNetApiHandler} object once and for the lifetime of this process.
*
* @param handler An instance that has not been initialized.
*/
public static void setHandler(SafeBrowsingApiHandler handler) {
public static void setHandler(SafetyNetApiHandler handler) {
synchronized (sLock) {
assert sHandler == null;
sHandler = handler;
Expand All @@ -68,10 +68,10 @@ public static void clearHandlerForTesting() {
}

/**
* Initializes the singleton SafeBrowsingApiHandler instance on the first call. On subsequent
* Initializes the singleton SafetyNetApiHandler instance on the first call. On subsequent
* calls it does nothing, returns the same value as returned on the first call.
*
* The caller must {@link #setHandler(SafeBrowsingApiHandler)} first.
* The caller must {@link #setHandler(SafetyNetApiHandler)} first.
*
* @return true iff the initialization succeeded.
*/
Expand All @@ -87,7 +87,7 @@ public static boolean ensureInitialized() {
*/
public interface UrlCheckTimeObserver {
/**
* @param urlCheckTimeDeltaMicros Time it took for {@link SafeBrowsingApiHandler} to check
* @param urlCheckTimeDeltaMicros Time it took for {@link SafetyNetApiHandler} to check
* the URL.
*/
void onUrlCheckTime(long urlCheckTimeDeltaMicros);
Expand All @@ -106,7 +106,7 @@ public static void setOneTimeUrlCheckObserver(UrlCheckTimeObserver observer) {
}

@GuardedBy("sLock")
private static SafeBrowsingApiHandler getHandler() {
private static SafetyNetApiHandler getHandler() {
if (!sHandlerInitCalled) {
sHandler = initHandler();
sHandlerInitCalled = true;
Expand All @@ -115,14 +115,14 @@ private static SafeBrowsingApiHandler getHandler() {
}

/**
* Initializes the SafeBrowsingApiHandler, if supported.
* Initializes the SafetyNetApiHandler, if supported.
*
* The caller must {@link #setHandler(SafeBrowsingApiHandler)} first.
* The caller must {@link #setHandler(SafetyNetApiHandler)} first.
*
* @return the handler if it is usable, or null if the API is not supported.
*/
@GuardedBy("sLock")
private static SafeBrowsingApiHandler initHandler() {
private static SafetyNetApiHandler initHandler() {
try (TraceEvent t = TraceEvent.scoped("SafeBrowsingApiBridge.initHandler")) {
if (DEBUG) {
Log.i(TAG, "initHandler");
Expand All @@ -132,7 +132,7 @@ private static SafeBrowsingApiHandler initHandler() {
}
}

private static class LookupDoneObserver implements SafeBrowsingApiHandler.Observer {
private static class LookupDoneObserver implements SafetyNetApiHandler.Observer {
@Override
public void onUrlCheckDone(
long callbackId, int resultStatus, String metadata, long checkDelta) {
Expand Down Expand Up @@ -181,8 +181,6 @@ private static void startUriLookup(long callbackId, String uri, int[] threatsOfI
* Must only be called if {@link #ensureInitialized()} returns true.
*
* @return true iff the uri is in the allowlist.
*
* TODO(crbug.com/995926): Make this call async.
*/
@CalledByNative
private static boolean startAllowlistLookup(String uri, int threatType) {
Expand Down

0 comments on commit a63ea2f

Please sign in to comment.