Skip to content

Commit

Permalink
[WebLayer] Remove special DSE permissions logic
Browse files Browse the repository at this point in the history
This matches chrome's behavior with the RevertDSEAutomaticPermissions
feature enabled.

Bug: 1261472
Change-Id: I108ddebe9627203e10567bd0e989e737f5b038d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3232177
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/main@{#934749}
  • Loading branch information
clarkduvall authored and Chromium LUCI CQ committed Oct 26, 2021
1 parent e504c76 commit b41240c
Show file tree
Hide file tree
Showing 19 changed files with 50 additions and 259 deletions.
3 changes: 1 addition & 2 deletions weblayer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,6 @@ source_set("weblayer_lib_base") {
"browser/cookie_manager_impl.h",
"browser/cookie_settings_factory.cc",
"browser/cookie_settings_factory.h",
"browser/default_search_engine.cc",
"browser/default_search_engine.h",
"browser/download_impl.cc",
"browser/download_impl.h",
"browser/download_manager_delegate_impl.cc",
Expand Down Expand Up @@ -819,6 +817,7 @@ if (is_android) {
":weblayer_lib_base",
"//base",
"//base/test:test_support",
"//components/content_settings/core/browser",
"//components/infobars/android",
"//components/infobars/content",
"//components/infobars/core",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,40 +7,31 @@
import static androidx.test.espresso.Espresso.onView;
import static androidx.test.espresso.action.ViewActions.click;
import static androidx.test.espresso.assertion.ViewAssertions.matches;
import static androidx.test.espresso.intent.Intents.intended;
import static androidx.test.espresso.intent.Intents.intending;
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.withText;

import static org.hamcrest.core.StringContains.containsString;

import android.app.Activity;
import android.app.Instrumentation.ActivityResult;
import android.content.Intent;
import android.os.RemoteException;
import android.provider.Settings;

import androidx.test.espresso.intent.Intents;
import androidx.test.espresso.intent.matcher.IntentMatchers;
import androidx.test.filters.SmallTest;

import org.hamcrest.Matcher;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.chromium.base.test.util.DisabledTest;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.weblayer.SettingsTestUtils;
import org.chromium.weblayer.SiteSettingsActivity;
import org.chromium.weblayer.TestWebLayer;
import org.chromium.weblayer.WebLayer;

/**
* Tests the behavior of the Site Settings UI.
*/
@RunWith(WebLayerJUnit4ClassRunner.class)
public class SiteSettingsTest {
// google.com is the default search engine, which should always be in the
// list of sites in "All Sites".
private static final String GOOGLE_URL = "https://www.google.com";
private static final String PROFILE_NAME = "DefaultProfile";

Expand All @@ -59,10 +50,12 @@ public void testSiteSettingsLaunches() throws InterruptedException {

@Test
@SmallTest
public void testAllSitesLaunches() throws InterruptedException {
public void testAllSitesLaunches() throws Exception {
mSettingsTestRule.launchActivity(
SiteSettingsActivity.createIntentForSiteSettingsCategoryList(
mSettingsTestRule.getContext(), PROFILE_NAME, /*isIncognito=*/false));
TestWebLayer.getTestWebLayer(mSettingsTestRule.getContext())
.grantLocationPermission(GOOGLE_URL);

onView(withText("All sites")).perform(click());

Expand Down Expand Up @@ -131,26 +124,17 @@ public void testSingleSiteClearPopupLaunches() throws InterruptedException {
@Test
@SmallTest
public void testSingleSiteLocationAccess() throws InterruptedException, RemoteException {
try {
Intents.init();
TestWebLayer testWebLayer =
TestWebLayer.getTestWebLayer(mSettingsTestRule.getContext());
testWebLayer.setSystemLocationSettingEnabled(true);
mSettingsTestRule.launchActivity(
SettingsTestUtils.createIntentForSiteSettingsSingleWebsite(
mSettingsTestRule.getContext(), PROFILE_NAME, /*isIncognito=*/false,
GOOGLE_URL));

Matcher<Intent> settingsMatcher =
IntentMatchers.hasAction(Settings.ACTION_APPLICATION_DETAILS_SETTINGS);
intending(settingsMatcher)
.respondWith(new ActivityResult(Activity.RESULT_OK, new Intent()));

onView(withText("Location")).perform(click());

intended(settingsMatcher);
} finally {
Intents.release();
}
TestWebLayer testWebLayer = TestWebLayer.getTestWebLayer(mSettingsTestRule.getContext());
testWebLayer.setSystemLocationSettingEnabled(true);
TestThreadUtils.runOnUiThreadBlocking(() -> {
WebLayer.loadSync(mSettingsTestRule.getContext()).getProfile(PROFILE_NAME);
});
testWebLayer.grantLocationPermission(GOOGLE_URL);
mSettingsTestRule.launchActivity(SettingsTestUtils.createIntentForSiteSettingsSingleWebsite(
mSettingsTestRule.getContext(), PROFILE_NAME, /*isIncognito=*/false, GOOGLE_URL));

onView(withText("Location")).perform(click());

onView(withText("Blocked")).check(matches(isDisplayed()));
}
}
7 changes: 0 additions & 7 deletions weblayer/browser/browser_context_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
#include "weblayer/browser/browsing_data_remover_delegate.h"
#include "weblayer/browser/browsing_data_remover_delegate_factory.h"
#include "weblayer/browser/client_hints_factory.h"
#include "weblayer/browser/default_search_engine.h"
#include "weblayer/browser/heavy_ad_service_factory.h"
#include "weblayer/browser/permissions/permission_manager_factory.h"
#include "weblayer/browser/stateful_ssl_host_state_delegate_factory.h"
Expand Down Expand Up @@ -118,12 +117,6 @@ BrowserContextImpl::BrowserContextImpl(ProfileImpl* profile_impl,
}

site_isolation::SiteIsolationPolicy::ApplyPersistedIsolatedOrigins(this);

// Set the DSE permissions every time the browser context is created for
// simplicity. These permissions are not editable in site settings, so should
// not ever be changed by the user. The site settings entry will link to the
// client app's system level permissions page to handle these.
ResetDsePermissions(this);
}

BrowserContextImpl::~BrowserContextImpl() {
Expand Down
4 changes: 0 additions & 4 deletions weblayer/browser/browsing_data_remover_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "content/public/browser/browsing_data_filter_builder.h"
#include "services/network/public/mojom/cookie_manager.mojom.h"
#include "weblayer/browser/browser_process.h"
#include "weblayer/browser/default_search_engine.h"
#include "weblayer/browser/favicon/favicon_service_impl.h"
#include "weblayer/browser/favicon/favicon_service_impl_factory.h"
#include "weblayer/browser/heavy_ad_service_factory.h"
Expand Down Expand Up @@ -112,9 +111,6 @@ void BrowsingDataRemoverDelegate::RemoveEmbedderData(
if (remove_mask & DATA_TYPE_SITE_SETTINGS) {
browsing_data::RemoveSiteSettingsData(delete_begin, delete_end,
host_content_settings_map);

// Reset the Default Search Engine permissions to their default.
ResetDsePermissions(browser_context_);
}

RunCallbackIfDone();
Expand Down
41 changes: 0 additions & 41 deletions weblayer/browser/default_search_engine.cc

This file was deleted.

39 changes: 0 additions & 39 deletions weblayer/browser/default_search_engine.h

This file was deleted.

54 changes: 0 additions & 54 deletions weblayer/browser/default_search_engine_browsertest.cc

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
import org.chromium.components.component_updater.EmbeddedComponentLoader;
import org.chromium.components.embedder_support.application.ClassLoaderContextWrapperFactory;
import org.chromium.components.embedder_support.application.FirebaseConfig;
import org.chromium.components.embedder_support.util.Origin;
import org.chromium.components.payments.PaymentDetailsUpdateService;
import org.chromium.content_public.browser.BrowserStartupController;
import org.chromium.content_public.browser.ChildProcessCreationParams;
Expand Down Expand Up @@ -639,13 +638,6 @@ public static String getClientApplicationName() {
.toString();
}

public static boolean isLocationPermissionManaged(Origin origin) {
if (origin == null) {
return false;
}
return WebLayerImplJni.get().isLocationPermissionManaged(origin.toString());
}

/**
* Converts the given id into a resource ID that can be shown in system UI, such as
* notifications.
Expand Down Expand Up @@ -1003,7 +995,6 @@ interface Natives {
void setIsWebViewCompatMode(boolean value);
String getUserAgentString();
void registerExternalExperimentIDs(int[] experimentIDs);
boolean isLocationPermissionManaged(String origin);
ComponentLoaderPolicyBridge[] getComponentLoaderPolicies();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import androidx.preference.Preference;

import org.chromium.base.Callback;
import org.chromium.base.ContextUtils;
import org.chromium.components.browser_ui.settings.ManagedPreferenceDelegate;
import org.chromium.components.browser_ui.site_settings.SiteSettingsCategory.Type;
import org.chromium.components.browser_ui.site_settings.SiteSettingsDelegate;
Expand Down Expand Up @@ -80,22 +79,12 @@ public String getAppName() {
@Override
@Nullable
public String getDelegateAppNameForOrigin(Origin origin, @ContentSettingsType int type) {
if (WebLayerImpl.isLocationPermissionManaged(origin)
&& type == ContentSettingsType.GEOLOCATION) {
return WebLayerImpl.getClientApplicationName();
}

return null;
}

@Override
@Nullable
public String getDelegatePackageNameForOrigin(Origin origin, @ContentSettingsType int type) {
if (WebLayerImpl.isLocationPermissionManaged(origin)
&& type == ContentSettingsType.GEOLOCATION) {
return ContextUtils.getApplicationContext().getPackageName();
}

return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,11 +331,18 @@ public void fireOnAccessTokenIdentifiedAsInvalid(IProfile profile,
profileImpl.fireOnAccessTokenIdentifiedAsInvalidForTesting(scopes, token);
}

@Override
public void grantLocationPermission(String url) {
TestThreadUtils.runOnUiThreadBlocking(
() -> { TestWebLayerImplJni.get().grantLocationPermission(url); });
}

@NativeMethods
interface Natives {
void waitForBrowserControlsMetadataState(
long tabImpl, int top, int bottom, Runnable runnable);
void setIgnoreMissingKeyForTranslateManager(boolean ignore);
void expediteDownloadService();
void grantLocationPermission(String url);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,7 @@ interface ITestWebLayer {
// Simulates the implementation-side event of an access token being
// identified as invalid.
void fireOnAccessTokenIdentifiedAsInvalid(in IProfile profile, in IObjectWrapper /* Set<String */ scopes, in IObjectWrapper /* String */ token) = 29;

// Grants `url` location permission.
void grantLocationPermission(String url) = 30;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "weblayer/browser/permissions/geolocation_permission_context_delegate.h"

#include "build/build_config.h"
#include "weblayer/browser/default_search_engine.h"

#if defined(OS_ANDROID)
#include "weblayer/browser/android/permission_request_utils.h"
Expand Down Expand Up @@ -40,8 +39,7 @@ PrefService* GeolocationPermissionContextDelegate::GetPrefs(
bool GeolocationPermissionContextDelegate::IsRequestingOriginDSE(
content::BrowserContext* browser_context,
const GURL& requesting_origin) {
return IsPermissionControlledByDse(ContentSettingsType::GEOLOCATION,
url::Origin::Create(requesting_origin));
return false;
}

void GeolocationPermissionContextDelegate::FinishNotifyPermissionSet(
Expand Down
Loading

0 comments on commit b41240c

Please sign in to comment.