Commit e1d6098f authored by Ehimare Okoyomon's avatar Ehimare Okoyomon Committed by Commit Bot

Add page info permissions observer for callbacks and metrics

Add an observer for SingleWebsiteSettings. Implement the callbacks to
record metrics and tell the PageInfoController to update permissions.

Bug: 1077766
Change-Id: I2d4f9d234b66a312372cf68e5d9f9a2cd49af798
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2431665Reviewed-by: default avatarBalazs Engedy <engedy@chromium.org>
Reviewed-by: default avatarChristian Dullweber <dullweber@chromium.org>
Commit-Queue: Ehimare Okoyomon <eokoyomon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812197}
parent 77a15429
...@@ -15,6 +15,8 @@ import static androidx.test.espresso.matcher.ViewMatchers.withText; ...@@ -15,6 +15,8 @@ import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.allOf;
import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
...@@ -23,6 +25,7 @@ import static org.chromium.components.content_settings.PrefNames.COOKIE_CONTROLS ...@@ -23,6 +25,7 @@ import static org.chromium.components.content_settings.PrefNames.COOKIE_CONTROLS
import android.os.Build; import android.os.Build;
import android.view.View; import android.view.View;
import android.widget.Button;
import androidx.test.filters.MediumTest; import androidx.test.filters.MediumTest;
...@@ -43,6 +46,7 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner; ...@@ -43,6 +46,7 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule; import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.browser_ui.site_settings.WebsitePreferenceBridge; import org.chromium.components.browser_ui.site_settings.WebsitePreferenceBridge;
import org.chromium.components.browser_ui.site_settings.WebsitePreferenceBridgeJni;
import org.chromium.components.content_settings.ContentSettingValues; import org.chromium.components.content_settings.ContentSettingValues;
import org.chromium.components.content_settings.ContentSettingsType; import org.chromium.components.content_settings.ContentSettingsType;
import org.chromium.components.content_settings.CookieControlsMode; import org.chromium.components.content_settings.CookieControlsMode;
...@@ -143,6 +147,20 @@ public class PageInfoViewTest { ...@@ -143,6 +147,20 @@ public class PageInfoViewTest {
}); });
} }
private void expectHasPermissions(String url, boolean hasPermissions) {
// The default value for these types is ask.
@ContentSettingValues
int expected = hasPermissions ? ContentSettingValues.ALLOW : ContentSettingValues.ASK;
TestThreadUtils.runOnUiThreadBlocking(() -> {
assertEquals(expected,
WebsitePreferenceBridgeJni.get().getNotificationSettingForOrigin(
Profile.getLastUsedRegularProfile(), url));
assertEquals(expected,
WebsitePreferenceBridgeJni.get().getGeolocationSettingForOrigin(
Profile.getLastUsedRegularProfile(), url, "*"));
});
}
private void addDefaultSettingPermissions(String url) { private void addDefaultSettingPermissions(String url) {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
WebsitePreferenceBridge.setContentSettingForPattern(Profile.getLastUsedRegularProfile(), WebsitePreferenceBridge.setContentSettingForPattern(Profile.getLastUsedRegularProfile(),
...@@ -299,20 +317,6 @@ public class PageInfoViewTest { ...@@ -299,20 +317,6 @@ public class PageInfoViewTest {
mRenderTestRule.render(getPageInfoView(), "PageInfo_ConnectionInfoSubpage"); mRenderTestRule.render(getPageInfoView(), "PageInfo_ConnectionInfoSubpage");
} }
/**
* Tests that the permissions page of the new PageInfo UI is gone when there are no permissions
* set.
*/
@Test
@MediumTest
@Features.EnableFeatures(PageInfoFeatureList.PAGE_INFO_V2)
public void testNoPermissionsSubpage() throws IOException {
loadUrlAndOpenPageInfo(mTestServerRule.getServer().getURL(sSimpleHtml));
View dialog = (View) getPageInfoView().getParent();
onView(withId(R.id.page_info_permissions_row))
.check(matches(withEffectiveVisibility(GONE)));
}
/** /**
* Tests the permissions page of the new PageInfo UI with permissions. * Tests the permissions page of the new PageInfo UI with permissions.
*/ */
...@@ -341,6 +345,20 @@ public class PageInfoViewTest { ...@@ -341,6 +345,20 @@ public class PageInfoViewTest {
mRenderTestRule.render(getPageInfoView(), "PageInfo_CookiesSubpage"); mRenderTestRule.render(getPageInfoView(), "PageInfo_CookiesSubpage");
} }
/**
* Tests that the permissions page of the new PageInfo UI is gone when there are no permissions
* set.
*/
@Test
@MediumTest
@Features.EnableFeatures(PageInfoFeatureList.PAGE_INFO_V2)
public void testNoPermissionsSubpage() throws IOException {
loadUrlAndOpenPageInfo(mTestServerRule.getServer().getURL(sSimpleHtml));
View dialog = (View) getPageInfoView().getParent();
onView(withId(R.id.page_info_permissions_row))
.check(matches(withEffectiveVisibility(GONE)));
}
/** /**
* Tests clearing cookies on the cookies page of the new PageInfo UI. * Tests clearing cookies on the cookies page of the new PageInfo UI.
*/ */
...@@ -366,5 +384,32 @@ public class PageInfoViewTest { ...@@ -366,5 +384,32 @@ public class PageInfoViewTest {
expectHasCookies(false); expectHasCookies(false);
} }
/**
* Tests resetting permissions on the permissions page of the new PageInfo UI.
*/
@Test
@MediumTest
@Features.EnableFeatures(PageInfoFeatureList.PAGE_INFO_V2)
public void testResetPermissionsOnSubpage() throws Exception {
mActivityTestRule.loadUrl(mTestServerRule.getServer().getURL(sSiteDataHtml));
String url = mTestServerRule.getServer().getURL("/");
// Create permissions.
expectHasPermissions(url, false);
addSomePermissions(url);
expectHasPermissions(url, true);
// Go to permissions subpage.
onView(withId(R.id.location_bar_status_icon)).perform(click());
onView(withId(R.id.page_info_permissions_row)).perform(click());
// Clear permissions in page info.
onView(withText("Reset permissions")).perform(click());
onView(allOf(is(instanceOf(Button.class)), withText("Reset permissions"))).perform(click());
// Wait until the UI navigates back and check permissions are reset.
onViewWaiting(allOf(withId(R.id.page_info_row_wrapper), isDisplayed()));
// Make sure that the permission section is gone because there are no longer exceptions.
onView(withId(R.id.page_info_permissions_row))
.check(matches(withEffectiveVisibility(GONE)));
expectHasPermissions(url, false);
}
// TODO(1071762): Add tests for preview pages, offline pages, offline state and other states. // TODO(1071762): Add tests for preview pages, offline pages, offline state and other states.
} }
...@@ -43,6 +43,21 @@ import java.util.Map; ...@@ -43,6 +43,21 @@ import java.util.Map;
*/ */
public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment
implements Preference.OnPreferenceChangeListener, Preference.OnPreferenceClickListener { implements Preference.OnPreferenceChangeListener, Preference.OnPreferenceClickListener {
/**
* Interface for a class that wants to receive updates from SingleWebsiteSettings.
*/
public interface Observer {
/**
* Notifies the observer that the website was reset.
*/
void onPermissionsReset();
/**
* Notifies the observer that a permission was changed.
*/
void onPermissionChanged();
}
// SingleWebsiteSettings expects either EXTRA_SITE (a Website) or // SingleWebsiteSettings expects either EXTRA_SITE (a Website) or
// EXTRA_SITE_ADDRESS (a WebsiteAddress) to be present (but not both). If // EXTRA_SITE_ADDRESS (a WebsiteAddress) to be present (but not both). If
// EXTRA_SITE is present, the fragment will display the permissions in that // EXTRA_SITE is present, the fragment will display the permissions in that
...@@ -131,7 +146,7 @@ public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment ...@@ -131,7 +146,7 @@ public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment
}; };
// The callback to be run after this site is reset. // The callback to be run after this site is reset.
private Runnable mWebsiteResetCallback; private Observer mWebsiteSettingsObserver;
private static final int REQUEST_CODE_NOTIFICATION_CHANNEL_SETTINGS = 1; private static final int REQUEST_CODE_NOTIFICATION_CHANNEL_SETTINGS = 1;
...@@ -260,8 +275,8 @@ public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment ...@@ -260,8 +275,8 @@ public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment
mHideNonPermissionPreferences = hide; mHideNonPermissionPreferences = hide;
} }
public void setWebsiteResetCallback(Runnable callback) { public void setWebsiteSettingsObserver(Observer observer) {
mWebsiteResetCallback = callback; mWebsiteSettingsObserver = observer;
} }
/** /**
...@@ -1010,6 +1025,9 @@ public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment ...@@ -1010,6 +1025,9 @@ public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment
} else { } else {
mSite.setPermission(browserContextHandle, type, permission); mSite.setPermission(browserContextHandle, type, permission);
} }
if (mWebsiteSettingsObserver != null) {
mWebsiteSettingsObserver.onPermissionChanged();
}
} }
return true; return true;
...@@ -1035,8 +1053,8 @@ public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment ...@@ -1035,8 +1053,8 @@ public class SingleWebsiteSettings extends SiteSettingsPreferenceFragment
} else { } else {
resetSite(); resetSite();
} }
if (mWebsiteResetCallback != null) { if (mWebsiteSettingsObserver != null) {
mWebsiteResetCallback.run(); mWebsiteSettingsObserver.onPermissionsReset();
} }
}) })
.setNegativeButton(R.string.cancel, null) .setNegativeButton(R.string.cancel, null)
......
...@@ -496,6 +496,15 @@ public class PageInfoController implements PageInfoMainController, ModalDialogPr ...@@ -496,6 +496,15 @@ public class PageInfoController implements PageInfoMainController, ModalDialogPr
} }
} }
@Override
public void refreshPermissions() {
mPermissionParamsListBuilder.clearPermissionEntries();
if (mNativePageInfoController != 0) {
PageInfoControllerJni.get().updatePermissions(
mNativePageInfoController, PageInfoController.this);
}
}
private boolean isSheet(Context context) { private boolean isSheet(Context context) {
return !DeviceFormFactor.isNonMultiDisplayContextOnTablet(context) return !DeviceFormFactor.isNonMultiDisplayContextOnTablet(context)
&& (mDelegate.getVrHandler() == null || !mDelegate.getVrHandler().isInVr()); && (mDelegate.getVrHandler() == null || !mDelegate.getVrHandler().isInVr());
...@@ -569,6 +578,7 @@ public class PageInfoController implements PageInfoMainController, ModalDialogPr ...@@ -569,6 +578,7 @@ public class PageInfoController implements PageInfoMainController, ModalDialogPr
void destroy(long nativePageInfoControllerAndroid, PageInfoController caller); void destroy(long nativePageInfoControllerAndroid, PageInfoController caller);
void recordPageInfoAction( void recordPageInfoAction(
long nativePageInfoControllerAndroid, PageInfoController caller, int action); long nativePageInfoControllerAndroid, PageInfoController caller, int action);
void updatePermissions(long nativePageInfoControllerAndroid, PageInfoController caller);
} }
@Override @Override
......
...@@ -28,6 +28,11 @@ public interface PageInfoMainController { ...@@ -28,6 +28,11 @@ public interface PageInfoMainController {
*/ */
void recordAction(@PageInfoAction int action); void recordAction(@PageInfoAction int action);
/**
* Refreshes the permissions of the page info.
*/
void refreshPermissions();
/** /**
* @return A BrowserContext for this dialog. * @return A BrowserContext for this dialog.
*/ */
......
...@@ -20,7 +20,8 @@ import java.util.List; ...@@ -20,7 +20,8 @@ import java.util.List;
/** /**
* Class for controlling the page info permissions section. * Class for controlling the page info permissions section.
*/ */
public class PageInfoPermissionsController implements PageInfoSubpageController { public class PageInfoPermissionsController
implements PageInfoSubpageController, SingleWebsiteSettings.Observer {
private PageInfoMainController mMainController; private PageInfoMainController mMainController;
private PageInfoRowView mRowView; private PageInfoRowView mRowView;
private PageInfoControllerDelegate mDelegate; private PageInfoControllerDelegate mDelegate;
...@@ -54,17 +55,12 @@ public class PageInfoPermissionsController implements PageInfoSubpageController ...@@ -54,17 +55,12 @@ public class PageInfoPermissionsController implements PageInfoSubpageController
mRowView.getContext(), SingleWebsiteSettings.class.getName(), fragmentArgs); mRowView.getContext(), SingleWebsiteSettings.class.getName(), fragmentArgs);
mSubpageFragment.setSiteSettingsClient(mDelegate.getSiteSettingsClient()); mSubpageFragment.setSiteSettingsClient(mDelegate.getSiteSettingsClient());
mSubpageFragment.setHideNonPermissionPreferences(true); mSubpageFragment.setHideNonPermissionPreferences(true);
mSubpageFragment.setWebsiteResetCallback(this::onPermissionsDeleted); mSubpageFragment.setWebsiteSettingsObserver(this);
AppCompatActivity host = (AppCompatActivity) mRowView.getContext(); AppCompatActivity host = (AppCompatActivity) mRowView.getContext();
host.getSupportFragmentManager().beginTransaction().add(mSubpageFragment, null).commitNow(); host.getSupportFragmentManager().beginTransaction().add(mSubpageFragment, null).commitNow();
return mSubpageFragment.requireView(); return mSubpageFragment.requireView();
} }
private void onPermissionsDeleted() {
mMainController.recordAction(PageInfoAction.PAGE_INFO_PERMISSIONS_CLEARED);
mMainController.exitSubpage();
}
@Override @Override
public void onSubpageRemoved() { public void onSubpageRemoved() {
assert mSubpageFragment != null; assert mSubpageFragment != null;
...@@ -139,4 +135,18 @@ public class PageInfoPermissionsController implements PageInfoSubpageController ...@@ -139,4 +135,18 @@ public class PageInfoPermissionsController implements PageInfoSubpageController
return resources.getQuantityString(resId, numPermissions - 2, perm1.name.toString(), return resources.getQuantityString(resId, numPermissions - 2, perm1.name.toString(),
perm2.name.toString(), numPermissions - 2); perm2.name.toString(), numPermissions - 2);
} }
// SingleWebsiteSettings.Observer methods
@Override
public void onPermissionsReset() {
mMainController.recordAction(PageInfoAction.PAGE_INFO_PERMISSIONS_CLEARED);
mMainController.refreshPermissions();
mMainController.exitSubpage();
}
@Override
public void onPermissionChanged() {
mMainController.recordAction(PageInfoAction.PAGE_INFO_PERMISSIONS_CHANGED);
}
} }
...@@ -77,6 +77,10 @@ public class PermissionParamsListBuilder { ...@@ -77,6 +77,10 @@ public class PermissionParamsListBuilder {
mEntries.add(new PageInfoPermissionEntry(name, type, value)); mEntries.add(new PageInfoPermissionEntry(name, type, value));
} }
public void clearPermissionEntries() {
mEntries.clear();
}
public PageInfoView.PermissionParams build() { public PageInfoView.PermissionParams build() {
List<PageInfoView.PermissionRowParams> rowParams = new ArrayList<>(); List<PageInfoView.PermissionRowParams> rowParams = new ArrayList<>();
for (PermissionParamsListBuilder.PageInfoPermissionEntry permission : mEntries) { for (PermissionParamsListBuilder.PageInfoPermissionEntry permission : mEntries) {
......
...@@ -80,6 +80,12 @@ void PageInfoControllerAndroid::RecordPageInfoAction( ...@@ -80,6 +80,12 @@ void PageInfoControllerAndroid::RecordPageInfoAction(
static_cast<PageInfo::PageInfoAction>(action)); static_cast<PageInfo::PageInfoAction>(action));
} }
void PageInfoControllerAndroid::UpdatePermissions(
JNIEnv* env,
const JavaParamRef<jobject>& obj) {
presenter_->UpdatePermissions();
}
void PageInfoControllerAndroid::SetIdentityInfo( void PageInfoControllerAndroid::SetIdentityInfo(
const IdentityInfo& identity_info) { const IdentityInfo& identity_info) {
JNIEnv* env = base::android::AttachCurrentThread(); JNIEnv* env = base::android::AttachCurrentThread();
......
...@@ -29,6 +29,8 @@ class PageInfoControllerAndroid : public PageInfoUI { ...@@ -29,6 +29,8 @@ class PageInfoControllerAndroid : public PageInfoUI {
void RecordPageInfoAction(JNIEnv* env, void RecordPageInfoAction(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj, const base::android::JavaParamRef<jobject>& obj,
jint action); jint action);
void UpdatePermissions(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
// PageInfoUI implementations. // PageInfoUI implementations.
void SetCookieInfo(const CookieInfoList& cookie_info_list) override; void SetCookieInfo(const CookieInfoList& cookie_info_list) override;
......
...@@ -513,6 +513,11 @@ void PageInfo::RecordPageInfoAction(PageInfoAction action) { ...@@ -513,6 +513,11 @@ void PageInfo::RecordPageInfoAction(PageInfoAction action) {
} }
} }
void PageInfo::UpdatePermissions() {
// Refresh the UI to reflect the new setting.
PresentSitePermissions();
}
void PageInfo::OnSitePermissionChanged(ContentSettingsType type, void PageInfo::OnSitePermissionChanged(ContentSettingsType type,
ContentSetting setting) { ContentSetting setting) {
ContentSettingChangedViaPageInfo(type); ContentSettingChangedViaPageInfo(type);
......
...@@ -139,6 +139,7 @@ class PageInfo : public content::WebContentsObserver { ...@@ -139,6 +139,7 @@ class PageInfo : public content::WebContentsObserver {
PAGE_INFO_COOKIES_CLEARED = 13, PAGE_INFO_COOKIES_CLEARED = 13,
PAGE_INFO_PERMISSION_DIALOG_OPENED = 14, PAGE_INFO_PERMISSION_DIALOG_OPENED = 14,
PAGE_INFO_PERMISSIONS_CLEARED = 15, PAGE_INFO_PERMISSIONS_CLEARED = 15,
PAGE_INFO_PERMISSIONS_CHANGED = 16,
PAGE_INFO_COUNT PAGE_INFO_COUNT
}; };
...@@ -200,6 +201,8 @@ class PageInfo : public content::WebContentsObserver { ...@@ -200,6 +201,8 @@ class PageInfo : public content::WebContentsObserver {
void RecordPageInfoAction(PageInfoAction action); void RecordPageInfoAction(PageInfoAction action);
void UpdatePermissions();
// This method is called when ever a permission setting is changed. // This method is called when ever a permission setting is changed.
void OnSitePermissionChanged(ContentSettingsType type, ContentSetting value); void OnSitePermissionChanged(ContentSettingsType type, ContentSetting value);
......
...@@ -75394,6 +75394,7 @@ Called by update_scheduler_enums.py.--> ...@@ -75394,6 +75394,7 @@ Called by update_scheduler_enums.py.-->
<int value="13" label="Cookies deleted for site"/> <int value="13" label="Cookies deleted for site"/>
<int value="14" label="Permission dialog opened"/> <int value="14" label="Permission dialog opened"/>
<int value="15" label="Permission deleted for site"/> <int value="15" label="Permission deleted for site"/>
<int value="16" label="Permission changed for site"/>
</enum> </enum>
<enum name="WebSiteSettingsAllSitesAction"> <enum name="WebSiteSettingsAllSitesAction">
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