diff options
author | miguelg@chromium.org <miguelg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-16 22:23:16 +0000 |
---|---|---|
committer | miguelg@chromium.org <miguelg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-16 22:23:16 +0000 |
commit | 3ce59a2f2be2b40e38f8e2ad75ba6f0476993c47 (patch) | |
tree | b30b89bbc3c17ba4edf1c79bcf8a09775ee34d1e | |
parent | 6ce1dd735b7c1ea85c23a8df814af2444caaeab2 (diff) | |
download | chromium_src-3ce59a2f2be2b40e38f8e2ad75ba6f0476993c47.zip chromium_src-3ce59a2f2be2b40e38f8e2ad75ba6f0476993c47.tar.gz chromium_src-3ce59a2f2be2b40e38f8e2ad75ba6f0476993c47.tar.bz2 |
Add UMA for the new generic permisison class
In order to track UMA accurately we only need to track it when there is an actual prompt for permission, not when the permission was saved for the domain. I have re-purposed the PermissionDecided method for this which was essentially doing nothing before.
BUG=392145
TBR=fgorski,tommi
Review URL: https://codereview.chromium.org/371933002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283546 0039d316-1c4b-4281-b951-d872f2087c98
18 files changed, 295 insertions, 38 deletions
diff --git a/chrome/browser/content_settings/content_settings_default_provider.cc b/chrome/browser/content_settings/content_settings_default_provider.cc index 8673960..c64a076 100644 --- a/chrome/browser/content_settings/content_settings_default_provider.cc +++ b/chrome/browser/content_settings/content_settings_default_provider.cc @@ -169,8 +169,11 @@ DefaultProvider::DefaultProvider(PrefService* prefs, bool incognito) ValueToContentSetting( default_settings_[CONTENT_SETTINGS_TYPE_MIDI_SYSEX].get()), CONTENT_SETTING_NUM_SETTINGS); - - // TODO(miguelg): Add UMA for Push messaging here + UMA_HISTOGRAM_ENUMERATION( + "ContentSettings.DefaultPushMessagingSetting", + ValueToContentSetting( + default_settings_[CONTENT_SETTINGS_TYPE_PUSH_MESSAGING].get()), + CONTENT_SETTING_NUM_SETTINGS); pref_change_registrar_.Init(prefs_); PrefChangeRegistrar::NamedChangeCallback callback = base::Bind( diff --git a/chrome/browser/content_settings/permission_bubble_request_impl.cc b/chrome/browser/content_settings/permission_bubble_request_impl.cc index 3767de2..b7341db 100644 --- a/chrome/browser/content_settings/permission_bubble_request_impl.cc +++ b/chrome/browser/content_settings/permission_bubble_request_impl.cc @@ -5,6 +5,7 @@ #include "chrome/browser/content_settings/permission_bubble_request_impl.h" #include "chrome/browser/content_settings/permission_context_base.h" +#include "chrome/browser/content_settings/permission_context_uma_util.h" #include "grit/generated_resources.h" #include "grit/theme_resources.h" #include "net/base/net_util.h" @@ -23,11 +24,14 @@ PermissionBubbleRequestImpl::PermissionBubbleRequestImpl( display_languages_(display_languages), permission_decided_callback_(permission_decided_callback), delete_callback_(delete_callback), - is_finished_(false) { + is_finished_(false), + action_taken_(false) { } PermissionBubbleRequestImpl::~PermissionBubbleRequestImpl() { DCHECK(is_finished_); + if (!action_taken_) + PermissionContextUmaUtil::PermissionIgnored(type_); } int PermissionBubbleRequestImpl::GetIconID() const { @@ -92,14 +96,17 @@ GURL PermissionBubbleRequestImpl::GetRequestingHostname() const { } void PermissionBubbleRequestImpl::PermissionGranted() { + RegisterActionTaken(); permission_decided_callback_.Run(true, true); } void PermissionBubbleRequestImpl::PermissionDenied() { + RegisterActionTaken(); permission_decided_callback_.Run(true, false); } void PermissionBubbleRequestImpl::Cancelled() { + RegisterActionTaken(); permission_decided_callback_.Run(false, false); } diff --git a/chrome/browser/content_settings/permission_bubble_request_impl.h b/chrome/browser/content_settings/permission_bubble_request_impl.h index fe73c64..89a9d60 100644 --- a/chrome/browser/content_settings/permission_bubble_request_impl.h +++ b/chrome/browser/content_settings/permission_bubble_request_impl.h @@ -18,7 +18,7 @@ class PermissionContextBase; // caller owns it and that it can be deleted once the |delete_callback| // is executed. class PermissionBubbleRequestImpl : public PermissionBubbleRequest { - public: + public: typedef base::Callback<void(bool persist_permission, bool grant_permission)> PermissionDecidedCallback; @@ -41,11 +41,17 @@ class PermissionBubbleRequestImpl : public PermissionBubbleRequest { // TODO(miguelg) Change this method to GetOrigin() virtual GURL GetRequestingHostname() const OVERRIDE; + + // Remember to call RegisterActionTaken for these methods if you are + // overriding them. virtual void PermissionGranted() OVERRIDE; virtual void PermissionDenied() OVERRIDE; virtual void Cancelled() OVERRIDE; virtual void RequestFinished() OVERRIDE; + protected: + void RegisterActionTaken() { action_taken_ = true; } + private: GURL request_origin_; bool user_gesture_; @@ -59,6 +65,7 @@ class PermissionBubbleRequestImpl : public PermissionBubbleRequest { // the caller. const base::Closure delete_callback_; bool is_finished_; + bool action_taken_; DISALLOW_COPY_AND_ASSIGN(PermissionBubbleRequestImpl); }; diff --git a/chrome/browser/content_settings/permission_context_base.cc b/chrome/browser/content_settings/permission_context_base.cc index 05504c5..265d8dc 100644 --- a/chrome/browser/content_settings/permission_context_base.cc +++ b/chrome/browser/content_settings/permission_context_base.cc @@ -8,6 +8,7 @@ #include "base/prefs/pref_service.h" #include "chrome/browser/content_settings/host_content_settings_map.h" #include "chrome/browser/content_settings/permission_bubble_request_impl.h" +#include "chrome/browser/content_settings/permission_context_uma_util.h" #include "chrome/browser/content_settings/permission_queue_controller.h" #include "chrome/browser/content_settings/permission_request_id.h" #include "chrome/browser/content_settings/tab_specific_content_settings.h" @@ -19,7 +20,6 @@ #include "grit/generated_resources.h" #include "grit/theme_resources.h" - PermissionContextBase::PermissionContextBase( Profile* profile, const ContentSettingsType permission_type) @@ -40,7 +40,6 @@ void PermissionContextBase::RequestPermission( const GURL& requesting_frame, bool user_gesture, const BrowserPermissionCallback& callback) { - // TODO(miguelg): Add UMA instrumentation. DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); DecidePermission(web_contents, @@ -65,16 +64,19 @@ void PermissionContextBase::DecidePermission( requesting_origin, embedder_origin, permission_type_, std::string()); switch (content_setting) { case CONTENT_SETTING_BLOCK: - PermissionDecided( - id, requesting_origin, embedder_origin, callback, false); + NotifyPermissionSet(id, requesting_origin, embedder_origin, + callback, false /* persist */, false /* granted */); return; case CONTENT_SETTING_ALLOW: - PermissionDecided(id, requesting_origin, embedder_origin, callback, true); + NotifyPermissionSet(id, requesting_origin, embedder_origin, + callback, false /* persist */, true /* granted */); return; default: break; } + PermissionContextUmaUtil::PermissionRequested(permission_type_); + if (PermissionBubbleManager::Enabled()) { PermissionBubbleManager* bubble_manager = PermissionBubbleManager::FromWebContents(web_contents); @@ -85,7 +87,7 @@ void PermissionContextBase::DecidePermission( user_gesture, permission_type_, profile_->GetPrefs()->GetString(prefs::kAcceptLanguages), - base::Bind(&PermissionContextBase::NotifyPermissionSet, + base::Bind(&PermissionContextBase::PermissionDecided, weak_factory_.GetWeakPtr(), id, requesting_origin, @@ -109,7 +111,7 @@ void PermissionContextBase::DecidePermission( requesting_origin, embedder_origin, std::string(), - base::Bind(&PermissionContextBase::NotifyPermissionSet, + base::Bind(&PermissionContextBase::PermissionDecided, weak_factory_.GetWeakPtr(), id, requesting_origin, @@ -125,10 +127,24 @@ void PermissionContextBase::PermissionDecided( const GURL& requesting_origin, const GURL& embedder_origin, const BrowserPermissionCallback& callback, + bool persist, bool allowed) { - DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + + // Infobar persistance and its related UMA is tracked on the infobar + // controller directly. + if (PermissionBubbleManager::Enabled()) { + if (persist) { + if (allowed) + PermissionContextUmaUtil::PermissionGranted(permission_type_); + else + PermissionContextUmaUtil::PermissionDenied(permission_type_); + } else { + PermissionContextUmaUtil::PermissionDismissed(permission_type_); + } + } + NotifyPermissionSet( - id, requesting_origin, embedder_origin, callback, false, allowed); + id, requesting_origin, embedder_origin, callback, persist, allowed); } PermissionQueueController* PermissionContextBase::GetQueueController() { diff --git a/chrome/browser/content_settings/permission_context_base.h b/chrome/browser/content_settings/permission_context_base.h index 76bbc91..07af561 100644 --- a/chrome/browser/content_settings/permission_context_base.h +++ b/chrome/browser/content_settings/permission_context_base.h @@ -79,6 +79,7 @@ class PermissionContextBase : public KeyedService { const GURL& requesting_origin, const GURL& embedder_origin, const BrowserPermissionCallback& callback, + bool persist, bool allowed); void NotifyPermissionSet(const PermissionRequestID& id, diff --git a/chrome/browser/content_settings/permission_context_uma_util.cc b/chrome/browser/content_settings/permission_context_uma_util.cc new file mode 100644 index 0000000..92da022 --- /dev/null +++ b/chrome/browser/content_settings/permission_context_uma_util.cc @@ -0,0 +1,112 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/metrics/histogram.h" +#include "chrome/browser/content_settings/permission_context_uma_util.h" + +namespace { + +// Enum for UMA purposes, make sure you update histograms.xml if you add new +// permission actions. Never delete or reorder an entry; only add new entries +// immediately before PERMISSION_NUM +enum PermissionAction { + GRANTED = 0, + DENIED = 1, + DISMISSED = 2, + IGNORED = 3, + + // Always keep this at the end. + PERMISSION_ACTION_NUM, +}; + +// Enum for UMA purposes, make sure you update histograms.xml if you add new +// permission actions. Never delete or reorder an entry; only add new entries +// immediately before PERMISSION_NUM +enum PermissionType { + PERMISSION_UNKNOWN, + PERMISSION_MIDI_SYSEX, + PERMISSION_PUSH_MESSAGING, + + // Always keep this at the end. + PERMISSION_NUM, +}; + +static const char* kMidiUmaKey = "ContentSettings.PermisionActions_MidiSysEx"; +static const char* kPushMessageUmaKey = + "ContentSettings.PermisionActions_PushMessaging"; + +void RecordPermissionAction( + ContentSettingsType permission, PermissionAction action) { + switch (permission) { + case CONTENT_SETTINGS_TYPE_GEOLOCATION: + // TODO(miguelg): support geolocation through + // the generic permission class. + break; + case CONTENT_SETTINGS_TYPE_MIDI_SYSEX: + UMA_HISTOGRAM_ENUMERATION(kMidiUmaKey, + action, + PERMISSION_ACTION_NUM); + break; + case CONTENT_SETTINGS_TYPE_PUSH_MESSAGING: + UMA_HISTOGRAM_ENUMERATION(kPushMessageUmaKey, + action, + PERMISSION_ACTION_NUM); + break; +#if defined(OS_ANDROID) + case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER: + // TODO(miguelg): support protected media through + // the generic permission class. + break; +#endif + default: + NOTREACHED() << "PERMISSION " << permission << " not accounted for"; + } +} + +void RecordPermissionRequest( + ContentSettingsType permission) { + PermissionType type; + switch (permission) { + case CONTENT_SETTINGS_TYPE_MIDI_SYSEX: + type = PERMISSION_MIDI_SYSEX; + break; + case CONTENT_SETTINGS_TYPE_PUSH_MESSAGING: + type = PERMISSION_PUSH_MESSAGING; + break; + default: + NOTREACHED() << "PERMISSION " << permission << " not accounted for"; + type = PERMISSION_UNKNOWN; + } + UMA_HISTOGRAM_ENUMERATION("ContentSettings.PermissionRequested", type, + PERMISSION_NUM); +} + +} // namespace + +// Make sure you update histograms.xml permission histogram_suffix if you +// add new permission +void PermissionContextUmaUtil::PermissionRequested( + ContentSettingsType permission) { + RecordPermissionRequest(permission); +} + +void PermissionContextUmaUtil::PermissionGranted( + ContentSettingsType permission) { + RecordPermissionAction(permission, GRANTED); +} + +void PermissionContextUmaUtil::PermissionDenied( + ContentSettingsType permission) { + RecordPermissionAction(permission, DENIED); +} + +void PermissionContextUmaUtil::PermissionDismissed( + ContentSettingsType permission) { + RecordPermissionAction(permission, DISMISSED); +} + +void PermissionContextUmaUtil::PermissionIgnored( + ContentSettingsType permission) { + RecordPermissionAction(permission, IGNORED); +} diff --git a/chrome/browser/content_settings/permission_context_uma_util.h b/chrome/browser/content_settings/permission_context_uma_util.h new file mode 100644 index 0000000..81deb57 --- /dev/null +++ b/chrome/browser/content_settings/permission_context_uma_util.h @@ -0,0 +1,25 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_CONTENT_SETTINGS_PERMISSION_CONTEXT_UMA_UTIL_H_ +#define CHROME_BROWSER_CONTENT_SETTINGS_PERMISSION_CONTEXT_UMA_UTIL_H_ + +#include "base/logging.h" +#include "chrome/common/content_settings_types.h" + +// Provides a convenient way of logging UMA for permission related +// operations. +class PermissionContextUmaUtil { + public: + static void PermissionGranted(ContentSettingsType permission); + static void PermissionDenied(ContentSettingsType permission); + static void PermissionDismissed(ContentSettingsType permission); + static void PermissionIgnored(ContentSettingsType permission); + static void PermissionRequested(ContentSettingsType permission); + + private: + DISALLOW_IMPLICIT_CONSTRUCTORS(PermissionContextUmaUtil); +}; + +#endif // CHROME_BROWSER_CONTENT_SETTINGS_PERMISSION_CONTEXT_UMA_UTIL_H_ diff --git a/chrome/browser/content_settings/permission_infobar_delegate.cc b/chrome/browser/content_settings/permission_infobar_delegate.cc index c200707..8d23b7d 100644 --- a/chrome/browser/content_settings/permission_infobar_delegate.cc +++ b/chrome/browser/content_settings/permission_infobar_delegate.cc @@ -4,19 +4,25 @@ #include "chrome/browser/content_settings/permission_infobar_delegate.h" +#include "chrome/browser/content_settings/permission_context_uma_util.h" #include "chrome/browser/content_settings/permission_queue_controller.h" #include "components/infobars/core/infobar.h" #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" PermissionInfobarDelegate::~PermissionInfobarDelegate() { + if (!action_taken_) + PermissionContextUmaUtil::PermissionIgnored(type_); } PermissionInfobarDelegate::PermissionInfobarDelegate( PermissionQueueController* controller, const PermissionRequestID& id, - const GURL& requesting_origin) - : controller_(controller), id_(id), requesting_origin_(requesting_origin) { + const GURL& requesting_origin, + ContentSettingsType type) + : controller_(controller), id_(id), requesting_origin_(requesting_origin), + action_taken_(false), + type_(type) { } void PermissionInfobarDelegate::InfoBarDismissed() { @@ -46,6 +52,7 @@ bool PermissionInfobarDelegate::Cancel() { void PermissionInfobarDelegate::SetPermission(bool update_content_setting, bool allowed) { + action_taken_ = true; controller_->OnPermissionSet( id_, requesting_origin_, InfoBarService::WebContentsFromInfoBar( diff --git a/chrome/browser/content_settings/permission_infobar_delegate.h b/chrome/browser/content_settings/permission_infobar_delegate.h index cdb8c20..ce838ce 100644 --- a/chrome/browser/content_settings/permission_infobar_delegate.h +++ b/chrome/browser/content_settings/permission_infobar_delegate.h @@ -7,6 +7,7 @@ #include "chrome/browser/content_settings/permission_request_id.h" #include "chrome/browser/infobars/infobar_service.h" +#include "chrome/common/content_settings_types.h" #include "components/infobars/core/confirm_infobar_delegate.h" #include "content/public/browser/web_contents.h" @@ -22,15 +23,19 @@ class PermissionInfobarDelegate : public ConfirmInfoBarDelegate { protected: PermissionInfobarDelegate(PermissionQueueController* controller, const PermissionRequestID& id, - const GURL& requesting_origin); + const GURL& requesting_origin, + ContentSettingsType type); virtual ~PermissionInfobarDelegate(); // ConfirmInfoBarDelegate: virtual base::string16 GetMessageText() const = 0; - virtual void InfoBarDismissed() OVERRIDE; virtual infobars::InfoBarDelegate::Type GetInfoBarType() const OVERRIDE; virtual base::string16 GetButtonLabel(InfoBarButton button) const OVERRIDE; + + // Remember to call RegisterActionTaken for these methods if you are + // overriding them. + virtual void InfoBarDismissed() OVERRIDE; virtual bool Accept() OVERRIDE; virtual bool Cancel() OVERRIDE; @@ -40,6 +45,8 @@ class PermissionInfobarDelegate : public ConfirmInfoBarDelegate { PermissionQueueController* controller_; // not owned by us const PermissionRequestID id_; GURL requesting_origin_; + bool action_taken_; + ContentSettingsType type_; DISALLOW_COPY_AND_ASSIGN(PermissionInfobarDelegate); }; diff --git a/chrome/browser/content_settings/permission_queue_controller.cc b/chrome/browser/content_settings/permission_queue_controller.cc index c0c395a..dd3de06 100644 --- a/chrome/browser/content_settings/permission_queue_controller.cc +++ b/chrome/browser/content_settings/permission_queue_controller.cc @@ -7,6 +7,7 @@ #include "base/prefs/pref_service.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/content_settings/host_content_settings_map.h" +#include "chrome/browser/content_settings/permission_context_uma_util.h" #include "chrome/browser/geolocation/geolocation_infobar_delegate.h" #include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/media/midi_permission_infobar_delegate.h" @@ -105,9 +106,6 @@ void PermissionQueueController::PendingInfobarRequest::RunCallback( void PermissionQueueController::PendingInfobarRequest::CreateInfoBar( PermissionQueueController* controller, const std::string& display_languages) { - // TODO(toyoshim): Remove following ContentType dependent code. - // Also these InfoBarDelegate can share much more code each other. - // http://crbug.com/266743 switch (type_) { case CONTENT_SETTINGS_TYPE_GEOLOCATION: infobar_ = GeolocationInfoBarDelegate::Create( @@ -117,12 +115,12 @@ void PermissionQueueController::PendingInfobarRequest::CreateInfoBar( case CONTENT_SETTINGS_TYPE_MIDI_SYSEX: infobar_ = MidiPermissionInfoBarDelegate::Create( GetInfoBarService(id_), controller, id_, requesting_frame_, - display_languages); + display_languages, type_); break; case CONTENT_SETTINGS_TYPE_PUSH_MESSAGING: infobar_ = gcm::PushMessagingInfoBarDelegate::Create( GetInfoBarService(id_), controller, id_, requesting_frame_, - display_languages); + display_languages, type_); break; #if defined(OS_ANDROID) case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER: @@ -194,8 +192,18 @@ void PermissionQueueController::OnPermissionSet( bool update_content_setting, bool allowed) { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); - if (update_content_setting) + + // TODO(miguelg): move the permission persistence to + // PermissionContextBase once all the types are moved there. + if (update_content_setting) { UpdateContentSetting(requesting_frame, embedder, allowed); + if (allowed) + PermissionContextUmaUtil::PermissionGranted(type_); + else + PermissionContextUmaUtil::PermissionDenied(type_); + } else { + PermissionContextUmaUtil::PermissionDismissed(type_); + } // Cancel this request first, then notify listeners. TODO(pkasting): Why // is this order important? diff --git a/chrome/browser/media/midi_permission_infobar_delegate.cc b/chrome/browser/media/midi_permission_infobar_delegate.cc index 394b284..f8b74a2 100644 --- a/chrome/browser/media/midi_permission_infobar_delegate.cc +++ b/chrome/browser/media/midi_permission_infobar_delegate.cc @@ -22,14 +22,15 @@ infobars::InfoBar* MidiPermissionInfoBarDelegate::Create( PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - const std::string& display_languages) { + const std::string& display_languages, + ContentSettingsType type) { const content::NavigationEntry* committed_entry = infobar_service->web_contents()->GetController().GetLastCommittedEntry(); return infobar_service->AddInfoBar(ConfirmInfoBarDelegate::CreateInfoBar( scoped_ptr<ConfirmInfoBarDelegate>(new MidiPermissionInfoBarDelegate( controller, id, requesting_frame, committed_entry ? committed_entry->GetUniqueID() : 0, - display_languages)))); + display_languages, type)))); } MidiPermissionInfoBarDelegate::MidiPermissionInfoBarDelegate( @@ -37,8 +38,9 @@ MidiPermissionInfoBarDelegate::MidiPermissionInfoBarDelegate( const PermissionRequestID& id, const GURL& requesting_frame, int contents_unique_id, - const std::string& display_languages) - : PermissionInfobarDelegate(controller, id, requesting_frame), + const std::string& display_languages, + ContentSettingsType type) + : PermissionInfobarDelegate(controller, id, requesting_frame, type), requesting_frame_(requesting_frame), display_languages_(display_languages) { } diff --git a/chrome/browser/media/midi_permission_infobar_delegate.h b/chrome/browser/media/midi_permission_infobar_delegate.h index 40aef21..02da941 100644 --- a/chrome/browser/media/midi_permission_infobar_delegate.h +++ b/chrome/browser/media/midi_permission_infobar_delegate.h @@ -6,8 +6,8 @@ #define CHROME_BROWSER_MEDIA_MIDI_PERMISSION_INFOBAR_DELEGATE_H_ #include <string> - #include "chrome/browser/content_settings/permission_infobar_delegate.h" +#include "chrome/common/content_settings_types.h" class GURL; class PermissionQueueController; @@ -24,14 +24,16 @@ class MidiPermissionInfoBarDelegate : public PermissionInfobarDelegate { PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - const std::string& display_languages); + const std::string& display_languages, + ContentSettingsType type); private: MidiPermissionInfoBarDelegate(PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, int contents_unique_id, - const std::string& display_languages); + const std::string& display_languages, + ContentSettingsType type); virtual ~MidiPermissionInfoBarDelegate(); // ConfirmInfoBarDelegate: diff --git a/chrome/browser/services/gcm/push_messaging_infobar_delegate.cc b/chrome/browser/services/gcm/push_messaging_infobar_delegate.cc index 454cb64..68a3978 100644 --- a/chrome/browser/services/gcm/push_messaging_infobar_delegate.cc +++ b/chrome/browser/services/gcm/push_messaging_infobar_delegate.cc @@ -18,20 +18,22 @@ infobars::InfoBar* PushMessagingInfoBarDelegate::Create( PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - const std::string& display_languages) { + const std::string& display_languages, + ContentSettingsType type) { DCHECK(infobar_service); DCHECK(controller); return infobar_service->AddInfoBar(ConfirmInfoBarDelegate::CreateInfoBar( scoped_ptr<ConfirmInfoBarDelegate>(new PushMessagingInfoBarDelegate( - controller, id, requesting_frame, display_languages)))); + controller, id, requesting_frame, display_languages, type)))); } PushMessagingInfoBarDelegate::PushMessagingInfoBarDelegate( PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - const std::string& display_languages) - : PermissionInfobarDelegate(controller, id, requesting_frame), + const std::string& display_languages, + ContentSettingsType type) + : PermissionInfobarDelegate(controller, id, requesting_frame, type), requesting_origin_(requesting_frame.GetOrigin()), display_languages_(display_languages) { } diff --git a/chrome/browser/services/gcm/push_messaging_infobar_delegate.h b/chrome/browser/services/gcm/push_messaging_infobar_delegate.h index 02d66e2..107a31d 100644 --- a/chrome/browser/services/gcm/push_messaging_infobar_delegate.h +++ b/chrome/browser/services/gcm/push_messaging_infobar_delegate.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_SERVICES_GCM_PUSH_MESSAGING_INFOBAR_DELEGATE_H_ #include "chrome/browser/content_settings/permission_infobar_delegate.h" +#include "chrome/common/content_settings_types.h" class GURL; class InfoBarService; @@ -22,13 +23,15 @@ class PushMessagingInfoBarDelegate : public PermissionInfobarDelegate { PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - const std::string& display_languages); + const std::string& display_languages, + ContentSettingsType type); private: PushMessagingInfoBarDelegate(PermissionQueueController* controller, const PermissionRequestID& id, const GURL& requesting_frame, - const std::string& display_languages); + const std::string& display_languages, + ContentSettingsType type); virtual ~PushMessagingInfoBarDelegate(); // ConfirmInfoBarDelegate: diff --git a/chrome/browser/ui/webui/options/content_settings_handler.cc b/chrome/browser/ui/webui/options/content_settings_handler.cc index 1db4886..145061e3 100644 --- a/chrome/browser/ui/webui/options/content_settings_handler.cc +++ b/chrome/browser/ui/webui/options/content_settings_handler.cc @@ -1366,6 +1366,10 @@ void ContentSettingsHandler::SetContentFilter(const base::ListValue* args) { content::RecordAction( UserMetricsAction("Options_DefaultMIDISysExSettingChanged")); break; + case CONTENT_SETTINGS_TYPE_PUSH_MESSAGING: + content::RecordAction( + UserMetricsAction("Options_DefaultPushMessagingSettingChanged")); + break; default: break; } diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index bf77d1c..ce1d414 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -336,6 +336,8 @@ 'browser/content_settings/permission_bubble_request_impl.h', 'browser/content_settings/permission_context_base.cc', 'browser/content_settings/permission_context_base.h', + 'browser/content_settings/permission_context_uma_util.cc', + 'browser/content_settings/permission_context_uma_util.h', 'browser/content_settings/permission_infobar_delegate.cc', 'browser/content_settings/permission_infobar_delegate.h', 'browser/content_settings/permission_queue_controller.cc', diff --git a/tools/metrics/actions/actions.xml b/tools/metrics/actions/actions.xml index 2053434..09901ae 100644 --- a/tools/metrics/actions/actions.xml +++ b/tools/metrics/actions/actions.xml @@ -8250,8 +8250,10 @@ should be able to be added at any place in this file. </action> <action name="Options_DefaultMIDISysExSettingChanged"> - <owner>Please list the metric's owners. Add more owner tags as needed.</owner> - <description>Please enter the description of this user action.</description> + <owner>toyoshim@chromium.org</owner> + <description> + Triggers when the default decision for Midi SysEx permissions is changed. + </description> </action> <action name="Options_DefaultMediaStreamMicSettingChanged"> @@ -8289,6 +8291,13 @@ should be able to be added at any place in this file. <description>Please enter the description of this user action.</description> </action> +<action name="Options_DefaultPushMessagingSettingChanged"> + <owner>miguelg@chromium.org</owner> + <description> + Triggers when the default decision for push message permissions is changed. + </description> +</action> + <action name="Options_DictionaryLanguage"> <owner>Please list the metric's owners. Add more owner tags as needed.</owner> <description>Please enter the description of this user action.</description> diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 8f60655..a1a3f29 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -2892,6 +2892,27 @@ Therefore, the affected-histogram name has to have at least one dot in it. <summary>The default popups setting at profile open.</summary> </histogram> +<histogram name="ContentSettings.DefaultPushMessagingSetting" + enum="ContentSetting"> + <owner>miguelg@chromium.org</owner> + <summary> + The default permission setting for push messages at profile open. + </summary> +</histogram> + +<histogram name="ContentSettings.PermissionActions" enum="PermissionAction"> + <owner>miguelg</owner> + <summary> + Tracks whether a permission was granted, rejected, etc. The suffix of the + histogram indicates which particular permission. + </summary> +</histogram> + +<histogram name="ContentSettings.PermissionRequested" enum="PermissionType"> + <owner>miguelg@chromium.org</owner> + <summary>Number of times a given permission was requested.</summary> +</histogram> + <histogram name="Cookie.ParsedCookieStatus" enum="ParsedCookieStatus"> <obsolete> Deprecated as of 9/2013. Experiment to measure control characters in cookies @@ -44224,6 +44245,19 @@ Therefore, the affected-histogram name has to have at least one dot in it. </int> </enum> +<enum name="PermissionAction" type="int"> + <int value="0" label="GRANTED"/> + <int value="1" label="DENIED"/> + <int value="2" label="DISMISSED"/> + <int value="3" label="IGNORED"/> +</enum> + +<enum name="PermissionType" type="int"> + <int value="0" label="PERMISSION_UNKONWN"/> + <int value="1" label="PERMISSION_MIDI_SYSEX"/> + <int value="2" label="PERMISSION_PUSH_MESSAGING"/> +</enum> + <enum name="PhotoEditorFileType" type="int"> <int value="0" label="jpg"/> <int value="1" label="png"/> @@ -50582,6 +50616,12 @@ Therefore, the affected-histogram name has to have at least one dot in it. <affected-histogram name="PerformanceMonitor.HighCPU"/> </histogram_suffixes> +<histogram_suffixes name="PermissionActions"> + <suffix name="MidiSysEx" label="Midi SysEx permsision actions"/> + <suffix name="PushMessaging" label="Push messaging permission actions"/> + <affected-histogram name="ContentSettings.PermissionActions"/> +</histogram_suffixes> + <histogram_suffixes name="PpapiPluginName"> <suffix name="libpepflashplayer.so" label="Flash player on Linux or Cros"/> <suffix name="libwidevinecdmadapter.so" |