Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(673)

Unified Diff: chrome/browser/about_flags.cc

Issue 344883002: Collect UMA statistics on which chrome://flags lead to chrome restart on ChromeOS. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Report hash(switch name). Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/about_flags.cc
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 7577556a0b3e3a97ca013d70cf3a88c938e8c25c..2acded706503d01281d6d208d2433b0cb435760d 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -11,6 +11,7 @@
#include "base/command_line.h"
#include "base/memory/singleton.h"
+#include "base/metrics/sparse_histogram.h"
#include "base/stl_util.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
@@ -29,6 +30,7 @@
#include "grit/generated_resources.h"
#include "grit/google_chrome_strings.h"
#include "media/base/media_switches.h"
+#include "third_party/zlib/zlib.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/ui_base_switches.h"
#include "ui/display/display_switches.h"
@@ -61,6 +63,8 @@ using base::UserMetricsAction;
namespace about_flags {
+const uint32_t kBadSwitchFormatHistogramId = 0;
+
// Macros to simplify specifying the type.
#define SINGLE_VALUE_TYPE_AND_VALUE(command_line_switch, switch_value) \
Experiment::SINGLE_VALUE, \
@@ -1514,17 +1518,8 @@ const Experiment kExperiments[] = {
IDS_FLAGS_ENABLE_ACCESSIBILITY_TAB_SWITCHER_NAME,
IDS_FLAGS_ENABLE_ACCESSIBILITY_TAB_SWITCHER_DESCRIPTION,
kOsAndroid,
- SINGLE_VALUE_TYPE(switches::kEnableAccessibilityTabSwitcher)
- },
- {
- // TODO(dmazzoni): remove this flag when native android accessibility
- // ships in the stable channel. http://crbug.com/356775
- "enable-accessibility-script-injection",
- IDS_FLAGS_ENABLE_ACCESSIBILITY_SCRIPT_INJECTION_NAME,
- IDS_FLAGS_ENABLE_ACCESSIBILITY_SCRIPT_INJECTION_DESCRIPTION,
- kOsAndroid,
- // Java-only switch: ContentSwitches.ENABLE_ACCESSIBILITY_SCRIPT_INJECTION.
- SINGLE_VALUE_TYPE("enable-accessibility-script-injection")
+ SINGLE_VALUE_TYPE(switches::kEnableAccessibilityTabSwitcher,
+ UMA_HISTOGRAM_ID_kEnableAccessibilityTabSwitcher)
sky 2014/08/05 16:49:14 Why are you doing this change?
Alexander Alekseev 2014/08/05 17:03:07 It is a bug. Thank you for pointing out. Fixed.
},
#endif
{
@@ -1954,6 +1949,7 @@ class FlagsState {
std::map<std::string, CommandLine::StringType>* switch_list);
void ResetAllFlags(FlagsStorage* flags_storage);
void reset();
+ std::set<std::string> GetAllSwitchesForTesting() const;
// Returns the singleton instance of this class
static FlagsState* GetInstance() {
@@ -2144,17 +2140,31 @@ void ConvertFlagsToSwitches(FlagsStorage* flags_storage,
}
bool AreSwitchesIdenticalToCurrentCommandLine(
- const CommandLine& new_cmdline, const CommandLine& active_cmdline) {
+ const CommandLine& new_cmdline,
+ const CommandLine& active_cmdline,
+ std::set<CommandLine::StringType>* out_difference) {
std::set<CommandLine::StringType> new_flags =
ExtractFlagsFromCommandLine(new_cmdline);
std::set<CommandLine::StringType> active_flags =
ExtractFlagsFromCommandLine(active_cmdline);
+ bool result = false;
// Needed because std::equal doesn't check if the 2nd set is empty.
- if (new_flags.size() != active_flags.size())
- return false;
+ if (new_flags.size() == active_flags.size()) {
+ result =
+ std::equal(new_flags.begin(), new_flags.end(), active_flags.begin());
+ }
- return std::equal(new_flags.begin(), new_flags.end(), active_flags.begin());
+ if (out_difference && !result) {
+ std::set_symmetric_difference(
+ new_flags.begin(),
+ new_flags.end(),
+ active_flags.begin(),
+ active_flags.end(),
+ std::inserter(*out_difference, out_difference->begin()));
+ }
+
+ return result;
}
void GetFlagsExperimentsData(FlagsStorage* flags_storage,
@@ -2262,6 +2272,47 @@ void RecordUMAStatistics(FlagsStorage* flags_storage) {
content::RecordAction(UserMetricsAction("StartupTick"));
}
+uint32 GetSwitchUMAId(const std::string& switch_name) {
+ return static_cast<unsigned>(
+ crc32(0,
+ reinterpret_cast<const Bytef*>(switch_name.c_str()),
+ switch_name.length()));
+}
+
+void ReportCustomFlags(const std::string& uma_histogram_hame,
+ const std::set<std::string>& command_line_difference) {
+ for (std::set<std::string>::const_iterator it =
+ command_line_difference.begin();
+ it != command_line_difference.end();
+ ++it) {
+ int uma_id = about_flags::kBadSwitchFormatHistogramId;
+ if (it->size() > 2) {
+ // Skip '--' before switch name.
+ std::string switch_name(it->substr(2));
+
+ // Kill value, if any.
+ const size_t value_pos = switch_name.find('=');
+ if (value_pos != std::string::npos)
+ switch_name.resize(value_pos);
+
+ uma_id = GetSwitchUMAId(switch_name);
+ } else {
+ NOTREACHED() << "ReportCustomFlags(): flag '" << *it
+ << "' has incorrect format.";
+ }
+ VLOG(1) << "ReportCustomFlags(): histogram='" << uma_histogram_hame << "' '"
+ << *it << "', uma_id=" << uma_id;
+
+ // Sparse histogram macro does not cache the histogram, so it's safe
+ // to use macro with non-static histogram name here.
+ UMA_HISTOGRAM_SPARSE_SLOWLY(uma_histogram_hame, uma_id);
+ }
+}
+
+std::set<std::string> GetAllSwitchesForTesting() {
+ return FlagsState::GetInstance()->GetAllSwitchesForTesting();
+}
+
//////////////////////////////////////////////////////////////////////////////
// FlagsState implementation.
@@ -2431,6 +2482,26 @@ void FlagsState::reset() {
flags_switches_.clear();
}
+std::set<std::string> FlagsState::GetAllSwitchesForTesting() const {
+ std::set<std::string> result;
+
+ for (size_t i = 0; i < num_experiments; ++i) {
+ const Experiment& e = experiments[i];
+ if (e.type == Experiment::SINGLE_VALUE) {
+ result.insert(e.command_line_switch);
+ } else if (e.type == Experiment::MULTI_VALUE) {
+ for (int j = 0; j < e.num_choices; ++j) {
+ result.insert(e.choices[j].command_line_switch);
+ }
+ } else {
+ DCHECK_EQ(e.type, Experiment::ENABLE_DISABLE_VALUE);
+ result.insert(e.command_line_switch);
+ result.insert(e.disable_command_line_switch);
+ }
+ }
+ return result;
+}
+
} // namespace
namespace testing {

Powered by Google App Engine
This is Rietveld 408576698