Chromium Code Reviews| Index: chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.cc |
| diff --git a/chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.cc b/chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.cc |
| index 9cd7a6eccfc571072da2d012d0fe2e709e108034..6fea114a486dffe13a7b0d61e4e0ec5cfbec392a 100644 |
| --- a/chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.cc |
| +++ b/chrome/browser/android/search_geolocation/search_geolocation_disclosure_tab_helper.cc |
| @@ -12,17 +12,19 @@ |
| #include "base/strings/string_number_conversions.h" |
| #include "base/time/time.h" |
| #include "chrome/browser/android/search_geolocation/search_geolocation_disclosure_infobar_delegate.h" |
| -#include "chrome/browser/permissions/permission_manager.h" |
| +#include "chrome/browser/content_settings/host_content_settings_map_factory.h" |
| #include "chrome/browser/profiles/profile.h" |
| #include "chrome/browser/search_engines/template_url_service_factory.h" |
| #include "chrome/common/chrome_features.h" |
| #include "chrome/common/pref_names.h" |
| +#include "components/content_settings/core/browser/host_content_settings_map.h" |
| +#include "components/content_settings/core/common/content_settings.h" |
| +#include "components/content_settings/core/common/content_settings_types.h" |
| #include "components/pref_registry/pref_registry_syncable.h" |
| #include "components/prefs/pref_service.h" |
| #include "components/search_engines/template_url.h" |
| #include "components/search_engines/template_url_service.h" |
| #include "components/variations/variations_associated_data.h" |
| -#include "content/public/browser/permission_type.h" |
| #include "content/public/browser/web_contents.h" |
| #include "jni/GeolocationHeader_jni.h" |
| #include "jni/SearchGeolocationDisclosureTabHelper_jni.h" |
| @@ -77,10 +79,13 @@ SearchGeolocationDisclosureTabHelper::~SearchGeolocationDisclosureTabHelper() {} |
| void SearchGeolocationDisclosureTabHelper::NavigationEntryCommitted( |
| const content::LoadCommittedDetails& load_details) { |
| - if (consistent_geolocation_enabled_) { |
| - MaybeShowDefaultSearchGeolocationDisclosure( |
| - web_contents()->GetVisibleURL()); |
| - } |
| + if (consistent_geolocation_enabled_) |
| + MaybeShowDisclosureForOmnibox(web_contents()->GetVisibleURL()); |
| +} |
| + |
| +void SearchGeolocationDisclosureTabHelper::MaybeShowDisclosureForAPIUsage( |
| + const GURL& gurl) { |
| + ShowDisclosureIfRulesPermit(gurl); |
|
raymes
2017/01/12 05:28:14
I guess we don't want to check the android permiss
benwells
2017/01/12 06:10:33
I thought about this last night and deliberately d
|
| } |
| // static |
| @@ -111,8 +116,8 @@ bool SearchGeolocationDisclosureTabHelper::Register(JNIEnv* env) { |
| return RegisterNativesImpl(env); |
| } |
| -void SearchGeolocationDisclosureTabHelper:: |
| - MaybeShowDefaultSearchGeolocationDisclosure(const GURL& gurl) { |
| +void SearchGeolocationDisclosureTabHelper::MaybeShowDisclosureForOmnibox( |
| + const GURL& gurl) { |
| // Don't show in incognito. |
| if (GetProfile()->IsOffTheRecord()) |
| return; |
| @@ -120,6 +125,16 @@ void SearchGeolocationDisclosureTabHelper:: |
| if (!ShouldShowDisclosureForUrl(gurl)) |
| return; |
| + // Check that the Chrome app has geolocation permission. |
| + JNIEnv* env = base::android::AttachCurrentThread(); |
| + if (!Java_GeolocationHeader_hasGeolocationPermission(env)) |
| + return; |
| + |
| + ShowDisclosureIfRulesPermit(gurl); |
| +} |
| + |
| +void SearchGeolocationDisclosureTabHelper::ShowDisclosureIfRulesPermit( |
| + const GURL& gurl) { |
| // Don't show the infobar if the user has dismissed it, or they've seen it |
| // enough times already. |
| PrefService* prefs = GetProfile()->GetPrefs(); |
| @@ -144,22 +159,17 @@ void SearchGeolocationDisclosureTabHelper:: |
| return; |
| } |
| - // Check that the Chrome app has geolocation permission. |
| - JNIEnv* env = base::android::AttachCurrentThread(); |
| - if (!Java_GeolocationHeader_hasGeolocationPermission(env)) |
| - return; |
| - |
| // Record metrics for the state of permissions before the disclosure has been |
| // shown. |
| RecordPreDisclosureMetrics(gurl); |
| - // Only show the disclosure if the geolocation permission is set to ASK |
| - // (i.e. has not been explicitly set or revoked). |
| - blink::mojom::PermissionStatus status = |
| - PermissionManager::Get(GetProfile()) |
| - ->GetPermissionStatus(content::PermissionType::GEOLOCATION, gurl, |
| - gurl); |
| - if (status != blink::mojom::PermissionStatus::ASK) |
| + // Only show the disclosure if the geolocation permission is set ask (i.e. has |
| + // not been explicitly set or revoked). |
| + ContentSetting status = |
| + HostContentSettingsMapFactory::GetForProfile(GetProfile()) |
| + ->GetContentSetting(gurl, gurl, CONTENT_SETTINGS_TYPE_GEOLOCATION, |
| + std::string()); |
| + if (status != CONTENT_SETTING_ASK) |
| return; |
| // All good, let's show the disclosure and increment the shown count. |
| @@ -201,15 +211,15 @@ void SearchGeolocationDisclosureTabHelper::RecordPreDisclosureMetrics( |
| PrefService* prefs = GetProfile()->GetPrefs(); |
| if (!prefs->GetBoolean( |
| prefs::kSearchGeolocationPreDisclosureMetricsRecorded)) { |
| - blink::mojom::PermissionStatus status = |
| - PermissionManager::Get(GetProfile()) |
| - ->GetPermissionStatus(content::PermissionType::GEOLOCATION, gurl, |
| - gurl); |
| - UMA_HISTOGRAM_ENUMERATION("GeolocationDisclosure.PreDisclosurePermission", |
| - static_cast<base::HistogramBase::Sample>(status), |
| - static_cast<base::HistogramBase::Sample>( |
| - blink::mojom::PermissionStatus::LAST) + |
| - 1); |
| + ContentSetting status = |
| + HostContentSettingsMapFactory::GetForProfile(GetProfile()) |
| + ->GetContentSetting(gurl, gurl, CONTENT_SETTINGS_TYPE_GEOLOCATION, |
| + std::string()); |
| + UMA_HISTOGRAM_ENUMERATION( |
| + "GeolocationDisclosure.PreDisclosureContentSetting", |
|
raymes
2017/01/12 05:28:14
Do we need to update histograms.xml?
benwells
2017/01/12 06:10:33
Done.
|
| + static_cast<base::HistogramBase::Sample>(status), |
| + static_cast<base::HistogramBase::Sample>(CONTENT_SETTING_NUM_SETTINGS) + |
| + 1); |
| prefs->SetBoolean(prefs::kSearchGeolocationPreDisclosureMetricsRecorded, |
| true); |
| } |
| @@ -220,15 +230,15 @@ void SearchGeolocationDisclosureTabHelper::RecordPostDisclosureMetrics( |
| PrefService* prefs = GetProfile()->GetPrefs(); |
| if (!prefs->GetBoolean( |
| prefs::kSearchGeolocationPostDisclosureMetricsRecorded)) { |
| - blink::mojom::PermissionStatus status = |
| - PermissionManager::Get(GetProfile()) |
| - ->GetPermissionStatus(content::PermissionType::GEOLOCATION, gurl, |
| - gurl); |
| - UMA_HISTOGRAM_ENUMERATION("GeolocationDisclosure.PostDisclosurePermission", |
| - static_cast<base::HistogramBase::Sample>(status), |
| - static_cast<base::HistogramBase::Sample>( |
| - blink::mojom::PermissionStatus::LAST) + |
| - 1); |
| + ContentSetting status = |
| + HostContentSettingsMapFactory::GetForProfile(GetProfile()) |
| + ->GetContentSetting(gurl, gurl, CONTENT_SETTINGS_TYPE_GEOLOCATION, |
| + std::string()); |
| + UMA_HISTOGRAM_ENUMERATION( |
| + "GeolocationDisclosure.PostDisclosureContentSetting", |
| + static_cast<base::HistogramBase::Sample>(status), |
| + static_cast<base::HistogramBase::Sample>(CONTENT_SETTING_NUM_SETTINGS) + |
| + 1); |
| prefs->SetBoolean(prefs::kSearchGeolocationPostDisclosureMetricsRecorded, |
| true); |
| } |