diff options
author | asvitkine@chromium.org <asvitkine@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-23 09:57:45 +0000 |
---|---|---|
committer | asvitkine@chromium.org <asvitkine@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-23 09:57:45 +0000 |
commit | 48ff2c7f5748aa73208040eb95ad88834fd3c474 (patch) | |
tree | 68cb0b2d5896ebfb99a8f8ee7f018b9f8bb3a715 | |
parent | 0aaa299b32073ff7b9971e28321949f38d45673d (diff) | |
download | chromium_src-48ff2c7f5748aa73208040eb95ad88834fd3c474.zip chromium_src-48ff2c7f5748aa73208040eb95ad88834fd3c474.tar.gz chromium_src-48ff2c7f5748aa73208040eb95ad88834fd3c474.tar.bz2 |
Create PluginMetricsProvider class.
Also changes ProvideStabilityMetrics() to take a SystemProfileProto* instead
of the stability section, since the plugin provider needs to inspect things
in the system profile when writing its stability data.
Also, cleans up some header includes.
BUG=374220
R=isherman@chromium.org, thestig@chromium.org
TBR=thestig@chromium.org
Review URL: https://codereview.chromium.org/299783004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272462 0039d316-1c4b-4281-b951-d872f2087c98
18 files changed, 615 insertions, 474 deletions
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index 91687ef..a4e35338 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -76,6 +76,7 @@ #include "chrome/common/switch_utils.h" #include "chrome/common/url_constants.h" #include "chrome/installer/util/google_update_constants.h" +#include "chrome/installer/util/google_update_settings.h" #include "components/policy/core/common/policy_service.h" #include "components/signin/core/common/profile_management_switches.h" #include "components/translate/core/browser/translate_download_manager.h" diff --git a/chrome/browser/metrics/android_metrics_provider.cc b/chrome/browser/metrics/android_metrics_provider.cc index f696b07..ddb4158 100644 --- a/chrome/browser/metrics/android_metrics_provider.cc +++ b/chrome/browser/metrics/android_metrics_provider.cc @@ -39,7 +39,7 @@ AndroidMetricsProvider::~AndroidMetricsProvider() { void AndroidMetricsProvider::ProvideStabilityMetrics( - metrics::SystemProfileProto_Stability* stability_proto) { + metrics::SystemProfileProto* system_profile_proto) { ConvertStabilityPrefsToHistograms(); } diff --git a/chrome/browser/metrics/android_metrics_provider.h b/chrome/browser/metrics/android_metrics_provider.h index 8a00adc..8b004da 100644 --- a/chrome/browser/metrics/android_metrics_provider.h +++ b/chrome/browser/metrics/android_metrics_provider.h @@ -25,7 +25,7 @@ class AndroidMetricsProvider : public metrics::MetricsProvider { // metrics::MetricsDataProvider: virtual void ProvideStabilityMetrics( - metrics::SystemProfileProto_Stability* stability_proto) OVERRIDE; + metrics::SystemProfileProto* system_profile_proto) OVERRIDE; // Registers local state prefs used by this class. static void RegisterPrefs(PrefRegistrySimple* registry); diff --git a/chrome/browser/metrics/chrome_stability_metrics_provider.cc b/chrome/browser/metrics/chrome_stability_metrics_provider.cc index 0af26ef..f846671 100644 --- a/chrome/browser/metrics/chrome_stability_metrics_provider.cc +++ b/chrome/browser/metrics/chrome_stability_metrics_provider.cc @@ -79,8 +79,10 @@ void ChromeStabilityMetricsProvider::OnRecordingDisabled() { } void ChromeStabilityMetricsProvider::ProvideStabilityMetrics( - metrics::SystemProfileProto_Stability* stability_proto) { + metrics::SystemProfileProto* system_profile_proto) { PrefService* pref = g_browser_process->local_state(); + metrics::SystemProfileProto_Stability* stability_proto = + system_profile_proto->mutable_stability(); int count = pref->GetInteger(prefs::kStabilityPageLoadCount); if (count) { diff --git a/chrome/browser/metrics/chrome_stability_metrics_provider.h b/chrome/browser/metrics/chrome_stability_metrics_provider.h index 246f7d7..9a814e0 100644 --- a/chrome/browser/metrics/chrome_stability_metrics_provider.h +++ b/chrome/browser/metrics/chrome_stability_metrics_provider.h @@ -29,7 +29,7 @@ class ChromeStabilityMetricsProvider : public metrics::MetricsProvider, virtual void OnRecordingEnabled() OVERRIDE; virtual void OnRecordingDisabled() OVERRIDE; virtual void ProvideStabilityMetrics( - metrics::SystemProfileProto_Stability* stability_proto) OVERRIDE; + metrics::SystemProfileProto* system_profile_proto) OVERRIDE; private: // content::NotificationObserver: diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc index f4ae797..b421c74 100644 --- a/chrome/browser/metrics/metrics_log.cc +++ b/chrome/browser/metrics/metrics_log.cc @@ -26,8 +26,6 @@ #include "base/tracked_objects.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/metrics/extension_metrics.h" -#include "chrome/browser/plugins/plugin_prefs.h" -#include "chrome/browser/profiles/profile_manager.h" #include "chrome/common/pref_names.h" #include "components/metrics/metrics_provider.h" #include "components/metrics/metrics_service_client.h" @@ -37,7 +35,6 @@ #include "components/variations/active_field_trials.h" #include "content/public/browser/gpu_data_manager.h" #include "content/public/common/content_client.h" -#include "content/public/common/webplugininfo.h" #include "gpu/config/gpu_info.h" #include "ui/gfx/screen.h" #include "url/gurl.h" @@ -116,38 +113,6 @@ std::string ComputeSHA1(const std::string& data) { return base::HexEncode(sha1.data(), sha1.size()); } -#if defined(ENABLE_PLUGINS) -// Returns the plugin preferences corresponding for this user, if available. -// If multiple user profiles are loaded, returns the preferences corresponding -// to an arbitrary one of the profiles. -PluginPrefs* GetPluginPrefs() { - ProfileManager* profile_manager = g_browser_process->profile_manager(); - - if (!profile_manager) { - // The profile manager can be NULL when testing. - return NULL; - } - - std::vector<Profile*> profiles = profile_manager->GetLoadedProfiles(); - if (profiles.empty()) - return NULL; - - return PluginPrefs::GetForProfile(profiles.front()).get(); -} - -// Fills |plugin| with the info contained in |plugin_info| and |plugin_prefs|. -void SetPluginInfo(const content::WebPluginInfo& plugin_info, - const PluginPrefs* plugin_prefs, - SystemProfileProto::Plugin* plugin) { - plugin->set_name(base::UTF16ToUTF8(plugin_info.name)); - plugin->set_filename(plugin_info.path.BaseName().AsUTF8Unsafe()); - plugin->set_version(base::UTF16ToUTF8(plugin_info.version)); - plugin->set_is_pepper(plugin_info.is_pepper_plugin()); - if (plugin_prefs) - plugin->set_is_disabled(!plugin_prefs->IsPluginEnabled(plugin_info)); -} -#endif // defined(ENABLE_PLUGINS) - void WriteFieldTrials(const std::vector<ActiveGroupId>& field_trial_ids, SystemProfileProto* system_profile) { for (std::vector<ActiveGroupId>::const_iterator it = @@ -280,11 +245,6 @@ MetricsLog::MetricsLog(const std::string& client_id, MetricsLog::~MetricsLog() {} -// static -void MetricsLog::RegisterPrefs(PrefRegistrySimple* registry) { - registry->RegisterListPref(prefs::kStabilityPluginStats); -} - void MetricsLog::RecordStabilityMetrics( const std::vector<metrics::MetricsProvider*>& metrics_providers, base::TimeDelta incremental_uptime, @@ -301,7 +261,6 @@ void MetricsLog::RecordStabilityMetrics( // sent, but that's true for all the metrics. WriteRequiredStabilityAttributes(pref); - WritePluginStabilityElements(pref); // Record recent delta for critical stability metrics. We can't wait for a // restart to gather these, as that delay biases our observation away from @@ -309,10 +268,9 @@ void MetricsLog::RecordStabilityMetrics( // uma log upload, just as we send histogram data. WriteRealtimeStabilityAttributes(pref, incremental_uptime, uptime); - SystemProfileProto::Stability* stability = - uma_proto()->mutable_system_profile()->mutable_stability(); + SystemProfileProto* system_profile = uma_proto()->mutable_system_profile(); for (size_t i = 0; i < metrics_providers.size(); ++i) - metrics_providers[i]->ProvideStabilityMetrics(stability); + metrics_providers[i]->ProvideStabilityMetrics(system_profile); // Omit some stats unless this is the initial stability log. if (log_type() != INITIAL_STABILITY_LOG) @@ -336,6 +294,8 @@ void MetricsLog::RecordStabilityMetrics( // TODO(jar): The following are all optional, so we *could* optimize them for // values of zero (and not include them). + SystemProfileProto::Stability* stability = + system_profile->mutable_stability(); stability->set_incomplete_shutdown_count(incomplete_shutdown_count); stability->set_breakpad_registration_success_count( breakpad_registration_success_count); @@ -382,76 +342,6 @@ bool MetricsLog::HasStabilityMetrics() const { return uma_proto()->system_profile().stability().has_launch_count(); } -void MetricsLog::WritePluginStabilityElements(PrefService* pref) { - // Now log plugin stability info. - const base::ListValue* plugin_stats_list = pref->GetList( - prefs::kStabilityPluginStats); - if (!plugin_stats_list) - return; - -#if defined(ENABLE_PLUGINS) - SystemProfileProto::Stability* stability = - uma_proto()->mutable_system_profile()->mutable_stability(); - for (base::ListValue::const_iterator iter = plugin_stats_list->begin(); - iter != plugin_stats_list->end(); ++iter) { - if (!(*iter)->IsType(base::Value::TYPE_DICTIONARY)) { - NOTREACHED(); - continue; - } - base::DictionaryValue* plugin_dict = - static_cast<base::DictionaryValue*>(*iter); - - // Note that this search is potentially a quadratic operation, but given the - // low number of plugins installed on a "reasonable" setup, this should be - // fine. - // TODO(isherman): Verify that this does not show up as a hotspot in - // profiler runs. - const SystemProfileProto::Plugin* system_profile_plugin = NULL; - std::string plugin_name; - plugin_dict->GetString(prefs::kStabilityPluginName, &plugin_name); - const SystemProfileProto& system_profile = uma_proto()->system_profile(); - for (int i = 0; i < system_profile.plugin_size(); ++i) { - if (system_profile.plugin(i).name() == plugin_name) { - system_profile_plugin = &system_profile.plugin(i); - break; - } - } - - if (!system_profile_plugin) { - NOTREACHED(); - continue; - } - - SystemProfileProto::Stability::PluginStability* plugin_stability = - stability->add_plugin_stability(); - *plugin_stability->mutable_plugin() = *system_profile_plugin; - - int launches = 0; - plugin_dict->GetInteger(prefs::kStabilityPluginLaunches, &launches); - if (launches > 0) - plugin_stability->set_launch_count(launches); - - int instances = 0; - plugin_dict->GetInteger(prefs::kStabilityPluginInstances, &instances); - if (instances > 0) - plugin_stability->set_instance_count(instances); - - int crashes = 0; - plugin_dict->GetInteger(prefs::kStabilityPluginCrashes, &crashes); - if (crashes > 0) - plugin_stability->set_crash_count(crashes); - - int loading_errors = 0; - plugin_dict->GetInteger(prefs::kStabilityPluginLoadingErrors, - &loading_errors); - if (loading_errors > 0) - plugin_stability->set_loading_error_count(loading_errors); - } -#endif // defined(ENABLE_PLUGINS) - - pref->ClearPref(prefs::kStabilityPluginStats); -} - // The server refuses data that doesn't have certain values. crashcount and // launchcount are currently "required" in the "stability" group. // TODO(isherman): Stop writing these attributes specially once the migration to @@ -497,25 +387,8 @@ void MetricsLog::WriteRealtimeStabilityAttributes( stability->set_uptime_sec(uptime_sec); } -void MetricsLog::WritePluginList( - const std::vector<content::WebPluginInfo>& plugin_list) { - DCHECK(!locked()); - -#if defined(ENABLE_PLUGINS) - PluginPrefs* plugin_prefs = GetPluginPrefs(); - SystemProfileProto* system_profile = uma_proto()->mutable_system_profile(); - for (std::vector<content::WebPluginInfo>::const_iterator iter = - plugin_list.begin(); - iter != plugin_list.end(); ++iter) { - SystemProfileProto::Plugin* plugin = system_profile->add_plugin(); - SetPluginInfo(*iter, plugin_prefs, plugin); - } -#endif // defined(ENABLE_PLUGINS) -} - void MetricsLog::RecordEnvironment( const std::vector<metrics::MetricsProvider*>& metrics_providers, - const std::vector<content::WebPluginInfo>& plugin_list, const std::vector<variations::ActiveGroupId>& synthetic_trials) { DCHECK(!HasEnvironment()); @@ -595,7 +468,6 @@ void MetricsLog::RecordEnvironment( WriteScreenDPIInformationProto(hardware); #endif - WritePluginList(plugin_list); extension_metrics_.WriteExtensionList(uma_proto()->mutable_system_profile()); std::vector<ActiveGroupId> field_trial_ids; diff --git a/chrome/browser/metrics/metrics_log.h b/chrome/browser/metrics/metrics_log.h index 177bf31..27f54a4 100644 --- a/chrome/browser/metrics/metrics_log.h +++ b/chrome/browser/metrics/metrics_log.h @@ -14,13 +14,11 @@ #include "base/basictypes.h" #include "chrome/browser/metrics/extension_metrics.h" #include "chrome/common/variations/variations_util.h" -#include "chrome/installer/util/google_update_settings.h" #include "components/metrics/metrics_log_base.h" #include "ui/gfx/size.h" class HashedExtensionMetrics; class PrefService; -class PrefRegistrySimple; #if defined(OS_CHROMEOS) class MetricsLogChromeOS; @@ -62,8 +60,6 @@ class MetricsLog : public metrics::MetricsLogBase { metrics::MetricsServiceClient* client); virtual ~MetricsLog(); - static void RegisterPrefs(PrefRegistrySimple* registry); - // Records the current operating environment, including metrics provided by // the specified set of |metrics_providers|. Takes the list of installed // plugins, Google Update statistics, and synthetic trial IDs as parameters @@ -73,7 +69,6 @@ class MetricsLog : public metrics::MetricsLogBase { // is determined by the pref value. void RecordEnvironment( const std::vector<metrics::MetricsProvider*>& metrics_providers, - const std::vector<content::WebPluginInfo>& plugin_list, const std::vector<variations::ActiveGroupId>& synthetic_trials); // Loads the environment proto that was saved by the last RecordEnvironment() @@ -145,9 +140,6 @@ class MetricsLog : public metrics::MetricsLogBase { // call to RecordStabilityMetrics(). bool HasStabilityMetrics() const; - // Within stability group, write plugin crash stats. - void WritePluginStabilityElements(PrefService* pref); - // Within the stability group, write required attributes. void WriteRequiredStabilityAttributes(PrefService* pref); @@ -159,9 +151,6 @@ class MetricsLog : public metrics::MetricsLogBase { base::TimeDelta incremental_uptime, base::TimeDelta uptime); - // Writes the list of installed plugins. - void WritePluginList(const std::vector<content::WebPluginInfo>& plugin_list); - // Used to interact with the embedder. Weak pointer; must outlive |this| // instance. metrics::MetricsServiceClient* const client_; diff --git a/chrome/browser/metrics/metrics_log_unittest.cc b/chrome/browser/metrics/metrics_log_unittest.cc index a2ae6f4..2a52719 100644 --- a/chrome/browser/metrics/metrics_log_unittest.cc +++ b/chrome/browser/metrics/metrics_log_unittest.cc @@ -11,12 +11,10 @@ #include "base/command_line.h" #include "base/port.h" #include "base/prefs/pref_service.h" -#include "base/prefs/scoped_user_pref_update.h" #include "base/prefs/testing_pref_service.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" -#include "base/strings/utf_string_conversions.h" #include "base/threading/sequenced_worker_pool.h" #include "base/time/time.h" #include "base/tracked_objects.h" @@ -24,17 +22,14 @@ #include "chrome/browser/prefs/browser_prefs.h" #include "chrome/common/chrome_version_info.h" #include "chrome/common/pref_names.h" -#include "chrome/installer/util/google_update_settings.h" #include "components/metrics/metrics_hashes.h" #include "components/metrics/metrics_provider.h" #include "components/metrics/proto/profiler_event.pb.h" #include "components/metrics/proto/system_profile.pb.h" #include "components/metrics/test_metrics_service_client.h" #include "components/variations/active_field_trials.h" -#include "components/variations/metrics_util.h" #include "content/public/browser/browser_thread.h" #include "content/public/common/process_type.h" -#include "content/public/common/webplugininfo.h" #include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_utils.h" #include "testing/gtest/include/gtest/gtest.h" @@ -99,24 +94,6 @@ const variations::ActiveGroupId kSyntheticTrials[] = { {66, 16} }; -#if defined(ENABLE_PLUGINS) -content::WebPluginInfo CreateFakePluginInfo( - const std::string& name, - const base::FilePath::CharType* path, - const std::string& version, - bool is_pepper) { - content::WebPluginInfo plugin(base::UTF8ToUTF16(name), - base::FilePath(path), - base::UTF8ToUTF16(version), - base::string16()); - if (is_pepper) - plugin.type = content::WebPluginInfo::PLUGIN_TYPE_PEPPER_IN_PROCESS; - else - plugin.type = content::WebPluginInfo::PLUGIN_TYPE_NPAPI; - return plugin; -} -#endif // defined(ENABLE_PLUGINS) - #if defined(OS_CHROMEOS) class TestMetricsLogChromeOS : public MetricsLogChromeOS { public: @@ -317,14 +294,13 @@ TEST_F(MetricsLogTest, RecordEnvironment) { metrics::TestMetricsServiceClient client; TestMetricsLog log(kClientId, kSessionId, MetricsLog::ONGOING_LOG, &client); - std::vector<content::WebPluginInfo> plugins; std::vector<variations::ActiveGroupId> synthetic_trials; // Add two synthetic trials. synthetic_trials.push_back(kSyntheticTrials[0]); synthetic_trials.push_back(kSyntheticTrials[1]); log.RecordEnvironment(std::vector<metrics::MetricsProvider*>(), - plugins, synthetic_trials); + synthetic_trials); // Check that the system profile on the log has the correct values set. CheckSystemProfile(log.system_profile()); @@ -361,7 +337,6 @@ TEST_F(MetricsLogTest, LoadSavedEnvironmentFromPrefs) { TestMetricsLog log( kClientId, kSessionId, MetricsLog::ONGOING_LOG, &client, &prefs); log.RecordEnvironment(std::vector<metrics::MetricsProvider*>(), - std::vector<content::WebPluginInfo>(), std::vector<variations::ActiveGroupId>()); EXPECT_FALSE(prefs.GetString(kSystemProfilePref).empty()); EXPECT_FALSE(prefs.GetString(kSystemProfileHashPref).empty()); @@ -385,7 +360,6 @@ TEST_F(MetricsLogTest, LoadSavedEnvironmentFromPrefs) { kClientId, kSessionId, MetricsLog::ONGOING_LOG, &client, &prefs); // Call RecordEnvironment() to record the pref again. log.RecordEnvironment(std::vector<metrics::MetricsProvider*>(), - std::vector<content::WebPluginInfo>(), std::vector<variations::ActiveGroupId>()); } @@ -407,7 +381,6 @@ TEST_F(MetricsLogTest, InitialLogStabilityMetrics) { kClientId, kSessionId, MetricsLog::INITIAL_STABILITY_LOG, &client); std::vector<metrics::MetricsProvider*> metrics_providers; log.RecordEnvironment(metrics_providers, - std::vector<content::WebPluginInfo>(), std::vector<variations::ActiveGroupId>()); log.RecordStabilityMetrics(metrics_providers, base::TimeDelta(), base::TimeDelta()); @@ -429,7 +402,6 @@ TEST_F(MetricsLogTest, OngoingLogStabilityMetrics) { TestMetricsLog log(kClientId, kSessionId, MetricsLog::ONGOING_LOG, &client); std::vector<metrics::MetricsProvider*> metrics_providers; log.RecordEnvironment(metrics_providers, - std::vector<content::WebPluginInfo>(), std::vector<variations::ActiveGroupId>()); log.RecordStabilityMetrics(metrics_providers, base::TimeDelta(), base::TimeDelta()); @@ -446,59 +418,6 @@ TEST_F(MetricsLogTest, OngoingLogStabilityMetrics) { EXPECT_FALSE(stability.has_debugger_not_present_count()); } -#if defined(ENABLE_PLUGINS) -TEST_F(MetricsLogTest, Plugins) { - metrics::TestMetricsServiceClient client; - TestMetricsLog log(kClientId, kSessionId, MetricsLog::ONGOING_LOG, &client); - - std::vector<metrics::MetricsProvider*> metrics_providers; - std::vector<content::WebPluginInfo> plugins; - plugins.push_back(CreateFakePluginInfo("p1", FILE_PATH_LITERAL("p1.plugin"), - "1.5", true)); - plugins.push_back(CreateFakePluginInfo("p2", FILE_PATH_LITERAL("p2.plugin"), - "2.0", false)); - log.RecordEnvironment(metrics_providers, plugins, - std::vector<variations::ActiveGroupId>()); - - const metrics::SystemProfileProto& system_profile = log.system_profile(); - ASSERT_EQ(2, system_profile.plugin_size()); - EXPECT_EQ("p1", system_profile.plugin(0).name()); - EXPECT_EQ("p1.plugin", system_profile.plugin(0).filename()); - EXPECT_EQ("1.5", system_profile.plugin(0).version()); - EXPECT_TRUE(system_profile.plugin(0).is_pepper()); - EXPECT_EQ("p2", system_profile.plugin(1).name()); - EXPECT_EQ("p2.plugin", system_profile.plugin(1).filename()); - EXPECT_EQ("2.0", system_profile.plugin(1).version()); - EXPECT_FALSE(system_profile.plugin(1).is_pepper()); - - // Now set some plugin stability stats for p2 and verify they're recorded. - scoped_ptr<base::DictionaryValue> plugin_dict(new base::DictionaryValue); - plugin_dict->SetString(prefs::kStabilityPluginName, "p2"); - plugin_dict->SetInteger(prefs::kStabilityPluginLaunches, 1); - plugin_dict->SetInteger(prefs::kStabilityPluginCrashes, 2); - plugin_dict->SetInteger(prefs::kStabilityPluginInstances, 3); - plugin_dict->SetInteger(prefs::kStabilityPluginLoadingErrors, 4); - { - ListPrefUpdate update(log.GetPrefService(), prefs::kStabilityPluginStats); - update.Get()->Append(plugin_dict.release()); - } - - log.RecordStabilityMetrics(metrics_providers, base::TimeDelta(), - base::TimeDelta()); - const metrics::SystemProfileProto_Stability& stability = - log.system_profile().stability(); - ASSERT_EQ(1, stability.plugin_stability_size()); - EXPECT_EQ("p2", stability.plugin_stability(0).plugin().name()); - EXPECT_EQ("p2.plugin", stability.plugin_stability(0).plugin().filename()); - EXPECT_EQ("2.0", stability.plugin_stability(0).plugin().version()); - EXPECT_FALSE(stability.plugin_stability(0).plugin().is_pepper()); - EXPECT_EQ(1, stability.plugin_stability(0).launch_count()); - EXPECT_EQ(2, stability.plugin_stability(0).crash_count()); - EXPECT_EQ(3, stability.plugin_stability(0).instance_count()); - EXPECT_EQ(4, stability.plugin_stability(0).loading_error_count()); -} -#endif // defined(ENABLE_PLUGINS) - // Test that we properly write profiler data to the log. TEST_F(MetricsLogTest, RecordProfilerData) { // WARNING: If you broke the below check, you've modified how @@ -694,9 +613,8 @@ TEST_F(MetricsLogTest, MultiProfileUserCount) { metrics::TestMetricsServiceClient client; TestMetricsLog log(kClientId, kSessionId, MetricsLog::ONGOING_LOG, &client); std::vector<metrics::MetricsProvider*> metrics_providers; - std::vector<content::WebPluginInfo> plugins; std::vector<variations::ActiveGroupId> synthetic_trials; - log.RecordEnvironment(metrics_providers, plugins, synthetic_trials); + log.RecordEnvironment(metrics_providers, synthetic_trials); EXPECT_EQ(2u, log.system_profile().multi_profile_user_count()); } @@ -721,9 +639,7 @@ TEST_F(MetricsLogTest, MultiProfileCountInvalidated) { user_manager->LoginUser(user2); std::vector<metrics::MetricsProvider*> metrics_providers; std::vector<variations::ActiveGroupId> synthetic_trials; - log.RecordEnvironment(metrics_providers, - std::vector<content::WebPluginInfo>(), - synthetic_trials); + log.RecordEnvironment(metrics_providers, synthetic_trials); EXPECT_EQ(0u, log.system_profile().multi_profile_user_count()); } @@ -731,7 +647,6 @@ TEST_F(MetricsLogTest, BluetoothHardwareDisabled) { metrics::TestMetricsServiceClient client; TestMetricsLog log(kClientId, kSessionId, MetricsLog::ONGOING_LOG, &client); log.RecordEnvironment(std::vector<metrics::MetricsProvider*>(), - std::vector<content::WebPluginInfo>(), std::vector<variations::ActiveGroupId>()); EXPECT_TRUE(log.system_profile().has_hardware()); @@ -750,7 +665,6 @@ TEST_F(MetricsLogTest, BluetoothHardwareEnabled) { metrics::TestMetricsServiceClient client; TestMetricsLog log(kClientId, kSessionId, MetricsLog::ONGOING_LOG, &client); log.RecordEnvironment(std::vector<metrics::MetricsProvider*>(), - std::vector<content::WebPluginInfo>(), std::vector<variations::ActiveGroupId>()); EXPECT_TRUE(log.system_profile().has_hardware()); @@ -781,7 +695,6 @@ TEST_F(MetricsLogTest, BluetoothPairedDevices) { metrics::TestMetricsServiceClient client; TestMetricsLog log(kClientId, kSessionId, MetricsLog::ONGOING_LOG, &client); log.RecordEnvironment(std::vector<metrics::MetricsProvider*>(), - std::vector<content::WebPluginInfo>(), std::vector<variations::ActiveGroupId>()); ASSERT_TRUE(log.system_profile().has_hardware()); diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc index 063f12f..9392702 100644 --- a/chrome/browser/metrics/metrics_service.cc +++ b/chrome/browser/metrics/metrics_service.cc @@ -174,7 +174,6 @@ #include "base/metrics/statistics_recorder.h" #include "base/prefs/pref_registry_simple.h" #include "base/prefs/pref_service.h" -#include "base/prefs/scoped_user_pref_update.h" #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" #include "base/threading/platform_thread.h" @@ -201,16 +200,17 @@ #include "components/metrics/metrics_reporting_scheduler.h" #include "components/metrics/metrics_service_client.h" #include "components/variations/entropy_provider.h" -#include "components/variations/metrics_util.h" #include "content/public/browser/child_process_data.h" -#include "content/public/browser/plugin_service.h" #include "content/public/browser/render_process_host.h" #include "content/public/browser/user_metrics.h" -#include "content/public/common/process_type.h" -#include "content/public/common/webplugininfo.h" #include "net/base/load_flags.h" #include "net/url_request/url_fetcher.h" +#if defined(ENABLE_PLUGINS) +// TODO(asvitkine): Move this out of MetricsService. +#include "chrome/browser/metrics/plugin_metrics_provider.h" +#endif + #if defined(OS_CHROMEOS) #include "chrome/browser/chromeos/settings/cros_settings.h" #include "chromeos/system/statistics_provider.h" @@ -229,8 +229,6 @@ using base::Time; using content::BrowserThread; -using content::ChildProcessData; -using content::PluginService; using metrics::MetricsLogManager; namespace { @@ -321,46 +319,6 @@ MetricsService::ShutdownCleanliness MetricsService::clean_shutdown_status_ = MetricsService::ExecutionPhase MetricsService::execution_phase_ = MetricsService::UNINITIALIZED_PHASE; -// This is used to quickly log stats from child process related notifications in -// MetricsService::child_stats_buffer_. The buffer's contents are transferred -// out when Local State is periodically saved. The information is then -// reported to the UMA server on next launch. -struct MetricsService::ChildProcessStats { - public: - explicit ChildProcessStats(int process_type) - : process_launches(0), - process_crashes(0), - instances(0), - loading_errors(0), - process_type(process_type) {} - - // This constructor is only used by the map to return some default value for - // an index for which no value has been assigned. - ChildProcessStats() - : process_launches(0), - process_crashes(0), - instances(0), - loading_errors(0), - process_type(content::PROCESS_TYPE_UNKNOWN) {} - - // The number of times that the given child process has been launched - int process_launches; - - // The number of times that the given child process has crashed - int process_crashes; - - // The number of instances of this child process that have been created. - // An instance is a DOM object rendered by this child process during a page - // load. - int instances; - - // The number of times there was an error loading an instance of this child - // process. - int loading_errors; - - int process_type; -}; - // static void MetricsService::RegisterPrefs(PrefRegistrySimple* registry) { DCHECK(IsSingleThreaded()); @@ -414,6 +372,11 @@ void MetricsService::RegisterPrefs(PrefRegistrySimple* registry) { // TODO(asvitkine): Move this out of here. AndroidMetricsProvider::RegisterPrefs(registry); #endif // defined(OS_ANDROID) + +#if defined(ENABLE_PLUGINS) + // TODO(asvitkine): Move this out of here. + PluginMetricsProvider::RegisterPrefs(registry); +#endif } MetricsService::MetricsService(metrics::MetricsStateManager* state_manager, @@ -458,6 +421,14 @@ MetricsService::MetricsService(metrics::MetricsStateManager* state_manager, RegisterMetricsProvider(scoped_ptr<metrics::MetricsProvider>( google_update_metrics_provider_)); #endif + +#if defined(ENABLE_PLUGINS) + plugin_metrics_provider_ = new PluginMetricsProvider( + g_browser_process->local_state()); + RegisterMetricsProvider(scoped_ptr<metrics::MetricsProvider>( + plugin_metrics_provider_)); +#endif + BrowserChildProcessObserver::Add(this); } @@ -592,23 +563,17 @@ void MetricsService::InconsistencyDetectedInLoggedCount(int amount) { std::abs(amount)); } -void MetricsService::BrowserChildProcessHostConnected( - const content::ChildProcessData& data) { - GetChildProcessStats(data).process_launches++; -} - void MetricsService::BrowserChildProcessCrashed( const content::ChildProcessData& data) { - GetChildProcessStats(data).process_crashes++; + // TODO(asvitkine): Move this into ChromeStabilityStatsProvider. +#if defined(ENABLE_PLUGINS) // Exclude plugin crashes from the count below because we report them via // a separate UMA metric. - if (!IsPluginProcess(data.process_type)) - IncrementPrefValue(prefs::kStabilityChildProcessCrashCount); -} + if (PluginMetricsProvider::IsPluginProcess(data.process_type)) + return; +#endif -void MetricsService::BrowserChildProcessInstanceCreated( - const content::ChildProcessData& data) { - GetChildProcessStats(data).instances++; + IncrementPrefValue(prefs::kStabilityChildProcessCrashCount); } void MetricsService::HandleIdleSinceLastTransmission(bool in_idle) { @@ -838,21 +803,19 @@ void MetricsService::OnInitTaskGotHardwareClass( DCHECK_EQ(INIT_TASK_SCHEDULED, state_); hardware_class_ = hardware_class; -#if defined(ENABLE_PLUGINS) - // Start the next part of the init task: loading plugin information. - PluginService::GetInstance()->GetPlugins( + const base::Closure got_plugin_info_callback = base::Bind(&MetricsService::OnInitTaskGotPluginInfo, - self_ptr_factory_.GetWeakPtr())); + self_ptr_factory_.GetWeakPtr()); + +#if defined(ENABLE_PLUGINS) + plugin_metrics_provider_->GetPluginInformation(got_plugin_info_callback); #else - std::vector<content::WebPluginInfo> plugin_list_empty; - OnInitTaskGotPluginInfo(plugin_list_empty); -#endif // defined(ENABLE_PLUGINS) + got_plugin_info_callback.Run(); +#endif } -void MetricsService::OnInitTaskGotPluginInfo( - const std::vector<content::WebPluginInfo>& plugins) { +void MetricsService::OnInitTaskGotPluginInfo() { DCHECK_EQ(INIT_TASK_SCHEDULED, state_); - plugins_ = plugins; const base::Closure got_metrics_callback = base::Bind(&MetricsService::OnInitTaskGotGoogleUpdateData, @@ -1019,8 +982,7 @@ void MetricsService::CloseCurrentLog() { DCHECK(current_log); std::vector<variations::ActiveGroupId> synthetic_trials; GetCurrentSyntheticFieldTrials(&synthetic_trials); - current_log->RecordEnvironment(metrics_providers_.get(), plugins_, - synthetic_trials); + current_log->RecordEnvironment(metrics_providers_.get(), synthetic_trials); PrefService* pref = g_browser_process->local_state(); base::TimeDelta incremental_uptime; base::TimeDelta uptime; @@ -1238,7 +1200,7 @@ void MetricsService::PrepareInitialMetricsLog() { std::vector<variations::ActiveGroupId> synthetic_trials; GetCurrentSyntheticFieldTrials(&synthetic_trials); - initial_metrics_log_->RecordEnvironment(metrics_providers_.get(), plugins_, + initial_metrics_log_->RecordEnvironment(metrics_providers_.get(), synthetic_trials); PrefService* pref = g_browser_process->local_state(); base::TimeDelta incremental_uptime; @@ -1532,120 +1494,10 @@ void MetricsService::LogChromeOSCrash(const std::string &crash_type) { #endif // OS_CHROMEOS void MetricsService::LogPluginLoadingError(const base::FilePath& plugin_path) { - content::WebPluginInfo plugin; - bool success = - content::PluginService::GetInstance()->GetPluginInfoByPath(plugin_path, - &plugin); - DCHECK(success); - ChildProcessStats& stats = child_process_stats_buffer_[plugin.name]; - // Initialize the type if this entry is new. - if (stats.process_type == content::PROCESS_TYPE_UNKNOWN) { - // The plug-in process might not actually of type PLUGIN (which means - // NPAPI), but we only care that it is *a* plug-in process. - stats.process_type = content::PROCESS_TYPE_PLUGIN; - } else { - DCHECK(IsPluginProcess(stats.process_type)); - } - stats.loading_errors++; -} - -MetricsService::ChildProcessStats& MetricsService::GetChildProcessStats( - const content::ChildProcessData& data) { - const base::string16& child_name = data.name; - if (!ContainsKey(child_process_stats_buffer_, child_name)) { - child_process_stats_buffer_[child_name] = - ChildProcessStats(data.process_type); - } - return child_process_stats_buffer_[child_name]; -} - -void MetricsService::RecordPluginChanges(PrefService* pref) { - ListPrefUpdate update(pref, prefs::kStabilityPluginStats); - base::ListValue* plugins = update.Get(); - DCHECK(plugins); - - for (base::ListValue::iterator value_iter = plugins->begin(); - value_iter != plugins->end(); ++value_iter) { - if (!(*value_iter)->IsType(base::Value::TYPE_DICTIONARY)) { - NOTREACHED(); - continue; - } - - base::DictionaryValue* plugin_dict = - static_cast<base::DictionaryValue*>(*value_iter); - std::string plugin_name; - plugin_dict->GetString(prefs::kStabilityPluginName, &plugin_name); - if (plugin_name.empty()) { - NOTREACHED(); - continue; - } - - // TODO(viettrungluu): remove conversions - base::string16 name16 = base::UTF8ToUTF16(plugin_name); - if (child_process_stats_buffer_.find(name16) == - child_process_stats_buffer_.end()) { - continue; - } - - ChildProcessStats stats = child_process_stats_buffer_[name16]; - if (stats.process_launches) { - int launches = 0; - plugin_dict->GetInteger(prefs::kStabilityPluginLaunches, &launches); - launches += stats.process_launches; - plugin_dict->SetInteger(prefs::kStabilityPluginLaunches, launches); - } - if (stats.process_crashes) { - int crashes = 0; - plugin_dict->GetInteger(prefs::kStabilityPluginCrashes, &crashes); - crashes += stats.process_crashes; - plugin_dict->SetInteger(prefs::kStabilityPluginCrashes, crashes); - } - if (stats.instances) { - int instances = 0; - plugin_dict->GetInteger(prefs::kStabilityPluginInstances, &instances); - instances += stats.instances; - plugin_dict->SetInteger(prefs::kStabilityPluginInstances, instances); - } - if (stats.loading_errors) { - int loading_errors = 0; - plugin_dict->GetInteger(prefs::kStabilityPluginLoadingErrors, - &loading_errors); - loading_errors += stats.loading_errors; - plugin_dict->SetInteger(prefs::kStabilityPluginLoadingErrors, - loading_errors); - } - - child_process_stats_buffer_.erase(name16); - } - - // Now go through and add dictionaries for plugins that didn't already have - // reports in Local State. - for (std::map<base::string16, ChildProcessStats>::iterator cache_iter = - child_process_stats_buffer_.begin(); - cache_iter != child_process_stats_buffer_.end(); ++cache_iter) { - ChildProcessStats stats = cache_iter->second; - - // Insert only plugins information into the plugins list. - if (!IsPluginProcess(stats.process_type)) - continue; - - // TODO(viettrungluu): remove conversion - std::string plugin_name = base::UTF16ToUTF8(cache_iter->first); - - base::DictionaryValue* plugin_dict = new base::DictionaryValue; - - plugin_dict->SetString(prefs::kStabilityPluginName, plugin_name); - plugin_dict->SetInteger(prefs::kStabilityPluginLaunches, - stats.process_launches); - plugin_dict->SetInteger(prefs::kStabilityPluginCrashes, - stats.process_crashes); - plugin_dict->SetInteger(prefs::kStabilityPluginInstances, - stats.instances); - plugin_dict->SetInteger(prefs::kStabilityPluginLoadingErrors, - stats.loading_errors); - plugins->Append(plugin_dict); - } - child_process_stats_buffer_.clear(); +#if defined(ENABLE_PLUGINS) + // TODO(asvitkine): Move this out of here. + plugin_metrics_provider_->LogPluginLoadingError(plugin_path); +#endif } bool MetricsService::ShouldLogEvents() { @@ -1668,12 +1520,8 @@ void MetricsService::RecordBooleanPrefValue(const char* path, bool value) { void MetricsService::RecordCurrentState(PrefService* pref) { pref->SetInt64(prefs::kStabilityLastTimestampSec, Time::Now().ToTimeT()); - RecordPluginChanges(pref); +#if defined(ENABLE_PLUGINS) + plugin_metrics_provider_->RecordPluginChanges(); +#endif } -// static -bool MetricsService::IsPluginProcess(int process_type) { - return (process_type == content::PROCESS_TYPE_PLUGIN || - process_type == content::PROCESS_TYPE_PPAPI_PLUGIN || - process_type == content::PROCESS_TYPE_PPAPI_BROKER); -} diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h index f229028..dbd91b4 100644 --- a/chrome/browser/metrics/metrics_service.h +++ b/chrome/browser/metrics/metrics_service.h @@ -39,6 +39,7 @@ class GoogleUpdateMetricsProviderWin; class MetricsReportingScheduler; class PrefService; class PrefRegistrySimple; +class PluginMetricsProvider; namespace base { class DictionaryValue; @@ -51,7 +52,6 @@ struct ActiveGroupId; } namespace content { -struct WebPluginInfo; } namespace metrics { @@ -171,12 +171,8 @@ class MetricsService virtual void InconsistencyDetectedInLoggedCount(int amount) OVERRIDE; // Implementation of content::BrowserChildProcessObserver - virtual void BrowserChildProcessHostConnected( - const content::ChildProcessData& data) OVERRIDE; virtual void BrowserChildProcessCrashed( const content::ChildProcessData& data) OVERRIDE; - virtual void BrowserChildProcessInstanceCreated( - const content::ChildProcessData& data) OVERRIDE; // This should be called when the application is not idle, i.e. the user seems // to be interacting with the application. @@ -274,8 +270,6 @@ class MetricsService NEED_TO_SHUTDOWN = ~CLEANLY_SHUTDOWN }; - struct ChildProcessStats; - typedef std::vector<SyntheticTrialGroup> SyntheticTrialGroups; // First part of the init task. Called on the FILE thread to load hardware @@ -287,10 +281,9 @@ class MetricsService // loading plugin information. void OnInitTaskGotHardwareClass(const std::string& hardware_class); - // Callback from PluginService::GetPlugins() that continues the init task by - // launching a task to gather Google Update statistics. - void OnInitTaskGotPluginInfo( - const std::vector<content::WebPluginInfo>& plugins); + // Called after the Plugin init task has been completed that continues the + // init task by launching a task to gather Google Update statistics. + void OnInitTaskGotPluginInfo(); // Called after GoogleUpdate init task has been completed that continues the // init task by loading profiler data. @@ -400,14 +393,6 @@ class MetricsService // Records that the browser was shut down cleanly. void LogCleanShutdown(); - // Returns reference to ChildProcessStats corresponding to |data|. - ChildProcessStats& GetChildProcessStats( - const content::ChildProcessData& data); - - // Saves plugin-related updates from the in-object buffer to Local State - // for retrieval next time we send a Profile log (generally next launch). - void RecordPluginChanges(PrefService* pref); - // Records state that should be periodically saved, like uptime and // buffered plugin stability statistics. void RecordCurrentState(PrefService* pref); @@ -418,10 +403,6 @@ class MetricsService // Sets the value of the specified path in prefs and schedules a save. void RecordBooleanPrefValue(const char* path, bool value); - // Returns true if process of type |type| should be counted as a plugin - // process, and false otherwise. - static bool IsPluginProcess(int process_type); - // Returns a list of synthetic field trials that were active for the entire // duration of the current log. void GetCurrentSyntheticFieldTrials( @@ -480,8 +461,9 @@ class MetricsService // empty string. std::string hardware_class_; - // The list of plugins which was retrieved on the file thread. - std::vector<content::WebPluginInfo> plugins_; +#if defined(ENABLE_PLUGINS) + PluginMetricsProvider* plugin_metrics_provider_; +#endif #if defined(OS_WIN) GoogleUpdateMetricsProviderWin* google_update_metrics_provider_; @@ -509,9 +491,6 @@ class MetricsService WindowMap window_map_; int next_window_id_; - // Buffer of child process notifications for quick access. - std::map<base::string16, ChildProcessStats> child_process_stats_buffer_; - // Weak pointers factory used to post task on different threads. All weak // pointers managed by this factory have the same lifetime as MetricsService. base::WeakPtrFactory<MetricsService> self_ptr_factory_; diff --git a/chrome/browser/metrics/metrics_service_unittest.cc b/chrome/browser/metrics/metrics_service_unittest.cc index 7985361..33aea72 100644 --- a/chrome/browser/metrics/metrics_service_unittest.cc +++ b/chrome/browser/metrics/metrics_service_unittest.cc @@ -16,8 +16,6 @@ #include "components/metrics/metrics_service_observer.h" #include "components/metrics/test_metrics_service_client.h" #include "components/variations/metrics_util.h" -#include "content/public/common/process_type.h" -#include "content/public/common/webplugininfo.h" #include "content/public/test/test_browser_thread_bundle.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/gfx/size.h" @@ -172,15 +170,6 @@ class TestMetricsServiceObserver : public MetricsServiceObserver { } // namespace -TEST_F(MetricsServiceTest, IsPluginProcess) { - EXPECT_TRUE( - MetricsService::IsPluginProcess(content::PROCESS_TYPE_PLUGIN)); - EXPECT_TRUE( - MetricsService::IsPluginProcess(content::PROCESS_TYPE_PPAPI_PLUGIN)); - EXPECT_FALSE( - MetricsService::IsPluginProcess(content::PROCESS_TYPE_GPU)); -} - TEST_F(MetricsServiceTest, InitialStabilityLogAfterCleanShutDown) { EnableMetricsReporting(); GetLocalState()->SetBoolean(prefs::kStabilityExitedCleanly, true); @@ -204,7 +193,6 @@ TEST_F(MetricsServiceTest, InitialStabilityLogAfterCrash) { metrics::TestMetricsServiceClient client; TestMetricsLog log("client", 1, &client); log.RecordEnvironment(std::vector<metrics::MetricsProvider*>(), - std::vector<content::WebPluginInfo>(), std::vector<variations::ActiveGroupId>()); // Record stability build time and version from previous session, so that diff --git a/chrome/browser/metrics/plugin_metrics_provider.cc b/chrome/browser/metrics/plugin_metrics_provider.cc new file mode 100644 index 0000000..e7633cf --- /dev/null +++ b/chrome/browser/metrics/plugin_metrics_provider.cc @@ -0,0 +1,350 @@ +// 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 "chrome/browser/metrics/plugin_metrics_provider.h" + +#include <string> + +#include "base/prefs/pref_registry_simple.h" +#include "base/prefs/pref_service.h" +#include "base/prefs/scoped_user_pref_update.h" +#include "base/stl_util.h" +#include "base/strings/utf_string_conversions.h" +#include "chrome/browser/browser_process.h" +#include "chrome/browser/plugins/plugin_prefs.h" +#include "chrome/browser/profiles/profile_manager.h" +#include "chrome/common/pref_names.h" +#include "components/metrics/proto/system_profile.pb.h" +#include "content/public/browser/child_process_data.h" +#include "content/public/browser/plugin_service.h" +#include "content/public/common/process_type.h" +#include "content/public/common/webplugininfo.h" + +namespace { + +// Returns the plugin preferences corresponding for this user, if available. +// If multiple user profiles are loaded, returns the preferences corresponding +// to an arbitrary one of the profiles. +PluginPrefs* GetPluginPrefs() { + ProfileManager* profile_manager = g_browser_process->profile_manager(); + + if (!profile_manager) { + // The profile manager can be NULL when testing. + return NULL; + } + + std::vector<Profile*> profiles = profile_manager->GetLoadedProfiles(); + if (profiles.empty()) + return NULL; + + return PluginPrefs::GetForProfile(profiles.front()).get(); +} + +// Fills |plugin| with the info contained in |plugin_info| and |plugin_prefs|. +void SetPluginInfo(const content::WebPluginInfo& plugin_info, + const PluginPrefs* plugin_prefs, + metrics::SystemProfileProto::Plugin* plugin) { + plugin->set_name(base::UTF16ToUTF8(plugin_info.name)); + plugin->set_filename(plugin_info.path.BaseName().AsUTF8Unsafe()); + plugin->set_version(base::UTF16ToUTF8(plugin_info.version)); + plugin->set_is_pepper(plugin_info.is_pepper_plugin()); + if (plugin_prefs) + plugin->set_is_disabled(!plugin_prefs->IsPluginEnabled(plugin_info)); +} + +} // namespace + +// This is used to quickly log stats from child process related notifications in +// PluginMetricsProvider::child_stats_buffer_. The buffer's contents are +// transferred out when Local State is periodically saved. The information is +// then reported to the UMA server on next launch. +struct PluginMetricsProvider::ChildProcessStats { + public: + explicit ChildProcessStats(int process_type) + : process_launches(0), + process_crashes(0), + instances(0), + loading_errors(0), + process_type(process_type) {} + + // This constructor is only used by the map to return some default value for + // an index for which no value has been assigned. + ChildProcessStats() + : process_launches(0), + process_crashes(0), + instances(0), + loading_errors(0), + process_type(content::PROCESS_TYPE_UNKNOWN) {} + + // The number of times that the given child process has been launched + int process_launches; + + // The number of times that the given child process has crashed + int process_crashes; + + // The number of instances of this child process that have been created. + // An instance is a DOM object rendered by this child process during a page + // load. + int instances; + + // The number of times there was an error loading an instance of this child + // process. + int loading_errors; + + int process_type; +}; + +PluginMetricsProvider::PluginMetricsProvider(PrefService* local_state) + : local_state_(local_state), + weak_ptr_factory_(this) { + DCHECK(local_state_); + + BrowserChildProcessObserver::Add(this); +} + +PluginMetricsProvider::~PluginMetricsProvider() { + BrowserChildProcessObserver::Remove(this); +} + +void PluginMetricsProvider::GetPluginInformation( + const base::Closure& done_callback) { + content::PluginService::GetInstance()->GetPlugins( + base::Bind(&PluginMetricsProvider::OnGotPlugins, + weak_ptr_factory_.GetWeakPtr(), + done_callback)); +} + +void PluginMetricsProvider::ProvideSystemProfileMetrics( + metrics::SystemProfileProto* system_profile_proto) { + PluginPrefs* plugin_prefs = GetPluginPrefs(); + for (size_t i = 0; i < plugins_.size(); ++i) { + SetPluginInfo(plugins_[i], plugin_prefs, + system_profile_proto->add_plugin()); + } +} + +void PluginMetricsProvider::ProvideStabilityMetrics( + metrics::SystemProfileProto* system_profile_proto) { + const base::ListValue* plugin_stats_list = local_state_->GetList( + prefs::kStabilityPluginStats); + if (!plugin_stats_list) + return; + + metrics::SystemProfileProto::Stability* stability = + system_profile_proto->mutable_stability(); + for (base::ListValue::const_iterator iter = plugin_stats_list->begin(); + iter != plugin_stats_list->end(); ++iter) { + if (!(*iter)->IsType(base::Value::TYPE_DICTIONARY)) { + NOTREACHED(); + continue; + } + base::DictionaryValue* plugin_dict = + static_cast<base::DictionaryValue*>(*iter); + + // Note that this search is potentially a quadratic operation, but given the + // low number of plugins installed on a "reasonable" setup, this should be + // fine. + // TODO(isherman): Verify that this does not show up as a hotspot in + // profiler runs. + const metrics::SystemProfileProto::Plugin* system_profile_plugin = NULL; + std::string plugin_name; + plugin_dict->GetString(prefs::kStabilityPluginName, &plugin_name); + for (int i = 0; i < system_profile_proto->plugin_size(); ++i) { + if (system_profile_proto->plugin(i).name() == plugin_name) { + system_profile_plugin = &system_profile_proto->plugin(i); + break; + } + } + + if (!system_profile_plugin) { + NOTREACHED(); + continue; + } + + metrics::SystemProfileProto::Stability::PluginStability* plugin_stability = + stability->add_plugin_stability(); + *plugin_stability->mutable_plugin() = *system_profile_plugin; + + int launches = 0; + plugin_dict->GetInteger(prefs::kStabilityPluginLaunches, &launches); + if (launches > 0) + plugin_stability->set_launch_count(launches); + + int instances = 0; + plugin_dict->GetInteger(prefs::kStabilityPluginInstances, &instances); + if (instances > 0) + plugin_stability->set_instance_count(instances); + + int crashes = 0; + plugin_dict->GetInteger(prefs::kStabilityPluginCrashes, &crashes); + if (crashes > 0) + plugin_stability->set_crash_count(crashes); + + int loading_errors = 0; + plugin_dict->GetInteger(prefs::kStabilityPluginLoadingErrors, + &loading_errors); + if (loading_errors > 0) + plugin_stability->set_loading_error_count(loading_errors); + } + + local_state_->ClearPref(prefs::kStabilityPluginStats); +} + +void PluginMetricsProvider::RecordPluginChanges() { + ListPrefUpdate update(local_state_, prefs::kStabilityPluginStats); + base::ListValue* plugins = update.Get(); + DCHECK(plugins); + + for (base::ListValue::iterator value_iter = plugins->begin(); + value_iter != plugins->end(); ++value_iter) { + if (!(*value_iter)->IsType(base::Value::TYPE_DICTIONARY)) { + NOTREACHED(); + continue; + } + + base::DictionaryValue* plugin_dict = + static_cast<base::DictionaryValue*>(*value_iter); + std::string plugin_name; + plugin_dict->GetString(prefs::kStabilityPluginName, &plugin_name); + if (plugin_name.empty()) { + NOTREACHED(); + continue; + } + + // TODO(viettrungluu): remove conversions + base::string16 name16 = base::UTF8ToUTF16(plugin_name); + if (child_process_stats_buffer_.find(name16) == + child_process_stats_buffer_.end()) { + continue; + } + + ChildProcessStats stats = child_process_stats_buffer_[name16]; + if (stats.process_launches) { + int launches = 0; + plugin_dict->GetInteger(prefs::kStabilityPluginLaunches, &launches); + launches += stats.process_launches; + plugin_dict->SetInteger(prefs::kStabilityPluginLaunches, launches); + } + if (stats.process_crashes) { + int crashes = 0; + plugin_dict->GetInteger(prefs::kStabilityPluginCrashes, &crashes); + crashes += stats.process_crashes; + plugin_dict->SetInteger(prefs::kStabilityPluginCrashes, crashes); + } + if (stats.instances) { + int instances = 0; + plugin_dict->GetInteger(prefs::kStabilityPluginInstances, &instances); + instances += stats.instances; + plugin_dict->SetInteger(prefs::kStabilityPluginInstances, instances); + } + if (stats.loading_errors) { + int loading_errors = 0; + plugin_dict->GetInteger(prefs::kStabilityPluginLoadingErrors, + &loading_errors); + loading_errors += stats.loading_errors; + plugin_dict->SetInteger(prefs::kStabilityPluginLoadingErrors, + loading_errors); + } + + child_process_stats_buffer_.erase(name16); + } + + // Now go through and add dictionaries for plugins that didn't already have + // reports in Local State. + for (std::map<base::string16, ChildProcessStats>::iterator cache_iter = + child_process_stats_buffer_.begin(); + cache_iter != child_process_stats_buffer_.end(); ++cache_iter) { + ChildProcessStats stats = cache_iter->second; + + // Insert only plugins information into the plugins list. + if (!IsPluginProcess(stats.process_type)) + continue; + + // TODO(viettrungluu): remove conversion + std::string plugin_name = base::UTF16ToUTF8(cache_iter->first); + + base::DictionaryValue* plugin_dict = new base::DictionaryValue; + + plugin_dict->SetString(prefs::kStabilityPluginName, plugin_name); + plugin_dict->SetInteger(prefs::kStabilityPluginLaunches, + stats.process_launches); + plugin_dict->SetInteger(prefs::kStabilityPluginCrashes, + stats.process_crashes); + plugin_dict->SetInteger(prefs::kStabilityPluginInstances, + stats.instances); + plugin_dict->SetInteger(prefs::kStabilityPluginLoadingErrors, + stats.loading_errors); + plugins->Append(plugin_dict); + } + child_process_stats_buffer_.clear(); +} + +void PluginMetricsProvider::LogPluginLoadingError( + const base::FilePath& plugin_path) { + content::WebPluginInfo plugin; + bool success = + content::PluginService::GetInstance()->GetPluginInfoByPath(plugin_path, + &plugin); + DCHECK(success); + ChildProcessStats& stats = child_process_stats_buffer_[plugin.name]; + // Initialize the type if this entry is new. + if (stats.process_type == content::PROCESS_TYPE_UNKNOWN) { + // The plug-in process might not actually be of type PLUGIN (which means + // NPAPI), but we only care that it is *a* plug-in process. + stats.process_type = content::PROCESS_TYPE_PLUGIN; + } else { + DCHECK(IsPluginProcess(stats.process_type)); + } + stats.loading_errors++; +} + +void PluginMetricsProvider::SetPluginsForTesting( + const std::vector<content::WebPluginInfo>& plugins) { + plugins_ = plugins; +} + +// static +bool PluginMetricsProvider::IsPluginProcess(int process_type) { + return (process_type == content::PROCESS_TYPE_PLUGIN || + process_type == content::PROCESS_TYPE_PPAPI_PLUGIN || + process_type == content::PROCESS_TYPE_PPAPI_BROKER); +} + +// static +void PluginMetricsProvider::RegisterPrefs(PrefRegistrySimple* registry) { + registry->RegisterListPref(prefs::kStabilityPluginStats); +} + +void PluginMetricsProvider::OnGotPlugins( + const base::Closure& done_callback, + const std::vector<content::WebPluginInfo>& plugins) { + plugins_ = plugins; + done_callback.Run(); +} + +PluginMetricsProvider::ChildProcessStats& +PluginMetricsProvider::GetChildProcessStats( + const content::ChildProcessData& data) { + const base::string16& child_name = data.name; + if (!ContainsKey(child_process_stats_buffer_, child_name)) { + child_process_stats_buffer_[child_name] = + ChildProcessStats(data.process_type); + } + return child_process_stats_buffer_[child_name]; +} + +void PluginMetricsProvider::BrowserChildProcessHostConnected( + const content::ChildProcessData& data) { + GetChildProcessStats(data).process_launches++; +} + +void PluginMetricsProvider::BrowserChildProcessCrashed( + const content::ChildProcessData& data) { + GetChildProcessStats(data).process_crashes++; +} + +void PluginMetricsProvider::BrowserChildProcessInstanceCreated( + const content::ChildProcessData& data) { + GetChildProcessStats(data).instances++; +} diff --git a/chrome/browser/metrics/plugin_metrics_provider.h b/chrome/browser/metrics/plugin_metrics_provider.h new file mode 100644 index 0000000..14ba222 --- /dev/null +++ b/chrome/browser/metrics/plugin_metrics_provider.h @@ -0,0 +1,95 @@ +// 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_METRICS_PLUGIN_METRICS_PROVIDER_H_ +#define CHROME_BROWSER_METRICS_PLUGIN_METRICS_PROVIDER_H_ + +#include <map> +#include <vector> + +#include "base/basictypes.h" +#include "base/memory/weak_ptr.h" +#include "base/strings/string16.h" +#include "components/metrics/metrics_provider.h" +#include "content/public/browser/browser_child_process_observer.h" + +namespace base { +class FilePath; +} + +namespace content { +struct WebPluginInfo; +} + +class PrefRegistrySimple; +class PrefService; + +// PluginMetricsProvider is responsible for adding out plugin information to +// the UMA proto. +class PluginMetricsProvider : public metrics::MetricsProvider, + public content::BrowserChildProcessObserver { + public: + explicit PluginMetricsProvider(PrefService* local_state); + virtual ~PluginMetricsProvider(); + + // Fetches plugin information data asynchronously and calls |done_callback| + // when done. + void GetPluginInformation(const base::Closure& done_callback); + + // metrics::MetricsDataProvider: + virtual void ProvideSystemProfileMetrics( + metrics::SystemProfileProto* system_profile_proto) OVERRIDE; + virtual void ProvideStabilityMetrics( + metrics::SystemProfileProto* system_profile_proto) OVERRIDE; + + // Saves plugin-related updates from the in-object buffer to Local State + // for retrieval next time we send a Profile log (generally next launch). + void RecordPluginChanges(); + + // Notifies the provider about an error loading the plugin at |plugin_path|. + void LogPluginLoadingError(const base::FilePath& plugin_path); + + // Sets this provider's list of plugins, exposed for testing. + void SetPluginsForTesting(const std::vector<content::WebPluginInfo>& plugins); + + // Returns true if process of type |type| should be counted as a plugin + // process, and false otherwise. + static bool IsPluginProcess(int process_type); + + // Registers local state prefs used by this class. + static void RegisterPrefs(PrefRegistrySimple* registry); + + private: + struct ChildProcessStats; + + // Receives the plugin list from the PluginService and calls |done_callback|. + void OnGotPlugins(const base::Closure& done_callback, + const std::vector<content::WebPluginInfo>& plugins); + + // Returns reference to ChildProcessStats corresponding to |data|. + ChildProcessStats& GetChildProcessStats( + const content::ChildProcessData& data); + + // content::BrowserChildProcessObserver: + virtual void BrowserChildProcessHostConnected( + const content::ChildProcessData& data) OVERRIDE; + virtual void BrowserChildProcessCrashed( + const content::ChildProcessData& data) OVERRIDE; + virtual void BrowserChildProcessInstanceCreated( + const content::ChildProcessData& data) OVERRIDE; + + PrefService* local_state_; + + // The list of plugins which was retrieved on the file thread. + std::vector<content::WebPluginInfo> plugins_; + + // Buffer of child process notifications for quick access. + std::map<base::string16, ChildProcessStats> child_process_stats_buffer_; + + base::WeakPtrFactory<PluginMetricsProvider> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(PluginMetricsProvider); +}; + +#endif // CHROME_BROWSER_METRICS_PLUGIN_METRICS_PROVIDER_H_ diff --git a/chrome/browser/metrics/plugin_metrics_provider_unittest.cc b/chrome/browser/metrics/plugin_metrics_provider_unittest.cc new file mode 100644 index 0000000..dc96191 --- /dev/null +++ b/chrome/browser/metrics/plugin_metrics_provider_unittest.cc @@ -0,0 +1,102 @@ +// 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 "chrome/browser/metrics/plugin_metrics_provider.h" + +#include <string> + +#include "base/basictypes.h" +#include "base/prefs/pref_service.h" +#include "base/prefs/scoped_user_pref_update.h" +#include "base/prefs/testing_pref_service.h" +#include "base/strings/utf_string_conversions.h" +#include "chrome/common/pref_names.h" +#include "components/metrics/proto/system_profile.pb.h" +#include "content/public/common/process_type.h" +#include "content/public/common/webplugininfo.h" +#include "content/public/test/test_browser_thread_bundle.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +content::WebPluginInfo CreateFakePluginInfo( + const std::string& name, + const base::FilePath::CharType* path, + const std::string& version, + bool is_pepper) { + content::WebPluginInfo plugin(base::UTF8ToUTF16(name), + base::FilePath(path), + base::UTF8ToUTF16(version), + base::string16()); + if (is_pepper) + plugin.type = content::WebPluginInfo::PLUGIN_TYPE_PEPPER_IN_PROCESS; + else + plugin.type = content::WebPluginInfo::PLUGIN_TYPE_NPAPI; + return plugin; +} + +} // namespace + +TEST(PluginMetricsProviderTest, IsPluginProcess) { + EXPECT_TRUE(PluginMetricsProvider::IsPluginProcess( + content::PROCESS_TYPE_PLUGIN)); + EXPECT_TRUE(PluginMetricsProvider::IsPluginProcess( + content::PROCESS_TYPE_PPAPI_PLUGIN)); + EXPECT_FALSE(PluginMetricsProvider::IsPluginProcess( + content::PROCESS_TYPE_GPU)); +} + +TEST(PluginMetricsProviderTest, Plugins) { + content::TestBrowserThreadBundle thread_bundle; + + TestingPrefServiceSimple prefs; + PluginMetricsProvider::RegisterPrefs(prefs.registry()); + PluginMetricsProvider provider(&prefs); + + std::vector<content::WebPluginInfo> plugins; + plugins.push_back(CreateFakePluginInfo("p1", FILE_PATH_LITERAL("p1.plugin"), + "1.5", true)); + plugins.push_back(CreateFakePluginInfo("p2", FILE_PATH_LITERAL("p2.plugin"), + "2.0", false)); + provider.SetPluginsForTesting(plugins); + + metrics::SystemProfileProto system_profile; + provider.ProvideSystemProfileMetrics(&system_profile); + + ASSERT_EQ(2, system_profile.plugin_size()); + EXPECT_EQ("p1", system_profile.plugin(0).name()); + EXPECT_EQ("p1.plugin", system_profile.plugin(0).filename()); + EXPECT_EQ("1.5", system_profile.plugin(0).version()); + EXPECT_TRUE(system_profile.plugin(0).is_pepper()); + EXPECT_EQ("p2", system_profile.plugin(1).name()); + EXPECT_EQ("p2.plugin", system_profile.plugin(1).filename()); + EXPECT_EQ("2.0", system_profile.plugin(1).version()); + EXPECT_FALSE(system_profile.plugin(1).is_pepper()); + + // Now set some plugin stability stats for p2 and verify they're recorded. + scoped_ptr<base::DictionaryValue> plugin_dict(new base::DictionaryValue); + plugin_dict->SetString(prefs::kStabilityPluginName, "p2"); + plugin_dict->SetInteger(prefs::kStabilityPluginLaunches, 1); + plugin_dict->SetInteger(prefs::kStabilityPluginCrashes, 2); + plugin_dict->SetInteger(prefs::kStabilityPluginInstances, 3); + plugin_dict->SetInteger(prefs::kStabilityPluginLoadingErrors, 4); + { + ListPrefUpdate update(&prefs, prefs::kStabilityPluginStats); + update.Get()->Append(plugin_dict.release()); + } + + provider.ProvideStabilityMetrics(&system_profile); + + const metrics::SystemProfileProto_Stability& stability = + system_profile.stability(); + ASSERT_EQ(1, stability.plugin_stability_size()); + EXPECT_EQ("p2", stability.plugin_stability(0).plugin().name()); + EXPECT_EQ("p2.plugin", stability.plugin_stability(0).plugin().filename()); + EXPECT_EQ("2.0", stability.plugin_stability(0).plugin().version()); + EXPECT_FALSE(stability.plugin_stability(0).plugin().is_pepper()); + EXPECT_EQ(1, stability.plugin_stability(0).launch_count()); + EXPECT_EQ(2, stability.plugin_stability(0).crash_count()); + EXPECT_EQ(3, stability.plugin_stability(0).instance_count()); + EXPECT_EQ(4, stability.plugin_stability(0).loading_error_count()); +} diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc index 9c50ef9..a63cf83 100644 --- a/chrome/browser/prefs/browser_prefs.cc +++ b/chrome/browser/prefs/browser_prefs.cc @@ -38,7 +38,6 @@ #include "chrome/browser/media/media_capture_devices_dispatcher.h" #include "chrome/browser/media/media_device_id_salt.h" #include "chrome/browser/media/media_stream_devices_controller.h" -#include "chrome/browser/metrics/metrics_log.h" #include "chrome/browser/metrics/metrics_service.h" #include "chrome/browser/metrics/variations/variations_service.h" #include "chrome/browser/net/http_server_properties_manager.h" @@ -232,7 +231,6 @@ void RegisterLocalState(PrefRegistrySimple* registry) { IntranetRedirectDetector::RegisterPrefs(registry); IOThread::RegisterPrefs(registry); KeywordEditorController::RegisterPrefs(registry); - MetricsLog::RegisterPrefs(registry); MetricsService::RegisterPrefs(registry); PrefProxyConfigTrackerImpl::RegisterPrefs(registry); ProfileInfoCache::RegisterPrefs(registry); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 0a7684a..711e45c 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1218,6 +1218,8 @@ 'browser/metrics/omnibox_metrics_provider.h', 'browser/metrics/perf_provider_chromeos.cc', 'browser/metrics/perf_provider_chromeos.h', + 'browser/metrics/plugin_metrics_provider.cc', + 'browser/metrics/plugin_metrics_provider.h', 'browser/metrics/thread_watcher.cc', 'browser/metrics/thread_watcher.h', 'browser/metrics/thread_watcher_android.cc', @@ -2863,6 +2865,7 @@ }, { # enable_plugins==0 'sources/': [ ['exclude', '^browser/guest_view/web_view/plugin_permission_helper'], + ['exclude', '^browser/metrics/plugin_metrics_provider'], ['exclude', '^browser/pepper_'], ['exclude', '^browser/plugins/'], ['exclude', '^browser/renderer_host/pepper/'], diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 6debf0a..8b0031b 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1087,6 +1087,7 @@ 'browser/metrics/metrics_log_unittest.cc', 'browser/metrics/metrics_service_unittest.cc', 'browser/metrics/metrics_state_manager_unittest.cc', + 'browser/metrics/plugin_metrics_provider_unittest.cc', 'browser/metrics/thread_watcher_unittest.cc', 'browser/metrics/thread_watcher_android_unittest.cc', 'browser/metrics/time_ticks_experiment_unittest.cc', @@ -2247,7 +2248,7 @@ ['exclude', '^browser/plugins/'], ], 'sources!': [ - 'browser/plugins/plugin_info_message_filter_unittest.cc', + 'browser/metrics/plugin_metrics_provider_unittest.cc', ], }], ['enable_printing!=1', { diff --git a/components/metrics/metrics_provider.h b/components/metrics/metrics_provider.h index 5e48543..76d4f44 100644 --- a/components/metrics/metrics_provider.h +++ b/components/metrics/metrics_provider.h @@ -34,7 +34,7 @@ class MetricsProvider { // directly into |stability_proto| fields or by logging stability histograms // via the UMA_STABILITY_HISTOGRAM_ENUMERATION() macro. virtual void ProvideStabilityMetrics( - SystemProfileProto_Stability* stability_proto) {} + SystemProfileProto* system_profile_proto) {} // Provides general metrics that are neither system profile nor stability // metrics. |