Clean up session crashed bubble field trial & switches.

chrome/browser/ui/views/session_crashed_bubble_view.cc

 // 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/ui/views/session_crashed_bubble_view.h"
 
 #include <stddef.h>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/command_line.h"
 #include "base/macros.h"
-#include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
-#include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/metrics/metrics_reporting_state.h"
 #include "chrome/browser/sessions/session_restore.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_list_observer.h"
-#include "chrome/browser/ui/startup/startup_browser_creator_impl.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/toolbar/app_menu_button.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
-#include "chrome/common/chrome_switches.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/installer/util/google_update_settings.h"
-#include "components/metrics/metrics_pref_names.h"
-#include "components/prefs/pref_service.h"
 #include "components/strings/grit/components_chromium_strings.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/button/checkbox.h"
 #include "ui/views/controls/label.h"
@@ skipping 10 matching lines @@
 
 // Fixed width of the column holding the description label of the bubble.
 const int kWidthOfDescriptionText = 320;
 
 // Distance between checkbox and the text to the right of it.
 const int kCheckboxTextDistance = 4;
 
 // The color of the text of the sub panel to offer UMA opt-in.
 const SkColor kTextColor = SkColorSetRGB(102, 102, 102);
 
-#if !defined(OS_CHROMEOS)
-// The Finch study name and group name that enables session crashed bubble UI.
-const char kEnableBubbleUIFinchName[] = "EnableSessionCrashedBubbleUI";
-const char kDisableBubbleUIGroupPrefix[] = "Disabled";
-#endif
-
 enum SessionCrashedBubbleHistogramValue {
   SESSION_CRASHED_BUBBLE_SHOWN,
   SESSION_CRASHED_BUBBLE_ERROR,
   SESSION_CRASHED_BUBBLE_RESTORED,
   SESSION_CRASHED_BUBBLE_ALREADY_UMA_OPTIN,
   SESSION_CRASHED_BUBBLE_UMA_OPTIN,
   SESSION_CRASHED_BUBBLE_HELP,
   SESSION_CRASHED_BUBBLE_IGNORED,
   SESSION_CRASHED_BUBBLE_OPTIN_BAR_SHOWN,
   SESSION_CRASHED_BUBBLE_MAX,
 };
 
 void RecordBubbleHistogramValue(SessionCrashedBubbleHistogramValue value) {
   UMA_HISTOGRAM_ENUMERATION(
       "SessionCrashed.Bubble", value, SESSION_CRASHED_BUBBLE_MAX);
 }
 
 // Whether or not the bubble UI should be used.
 bool IsBubbleUIEnabled() {

[msw, 2016/10/07 16:54:29]

q: Should http://crbug.com/653287 also cover the removal of this and the infobar
UI? Otherwise, could you file another bug to track that work? Please add a TODO
here citing the appropriate issue.

[Alexei Svitkine (slow), 2016/10/07 17:33:19]

Done. File http://crbug.com/653966

 // Function ChangeMetricsReportingState (called when the user chooses to

[msw, 2016/10/07 16:54:29]

q: Should this really prevent using the bubble UI from showing altogether? Could
Chrome OS just show the bubble without the checkbox?

[Alexei Svitkine (slow), 2016/10/07 17:33:20]

I agree it should be enabled. I don't have cycles for this in the short term
however, so including this in the bug above.

 // opt-in to UMA) does not support Chrome OS yet, so don't show the bubble on
 // Chrome OS.
 #if defined(OS_CHROMEOS)
   return false;
 #else
-  const base::CommandLine& command_line =
-      *base::CommandLine::ForCurrentProcess();
-  if (command_line.HasSwitch(switches::kDisableSessionCrashedBubble))
-    return false;
-  if (command_line.HasSwitch(switches::kEnableSessionCrashedBubble))
-    return true;
-  const std::string group_name = base::FieldTrialList::FindFullName(
-      kEnableBubbleUIFinchName);
-
-  // When |group_name| starts with |kDisableBubbleUIGroupPrefix|, disable the
-  // bubble UI. I.e. the default behavior is bubble enabled unless overridden.
-  // This is to accommodate potential new group names without needing to change
-  // the code here.
-  return !base::StartsWith(group_name, kDisableBubbleUIGroupPrefix,
-                           base::CompareCase::SENSITIVE);
+  return true;
 #endif
 }
 
 }  // namespace
 
 // A helper class that listens to browser removal event.
 class SessionCrashedBubbleView::BrowserRemovalObserver
     : public chrome::BrowserListObserver {
  public:
   explicit BrowserRemovalObserver(Browser* browser) : browser_(browser) {
@@ skipping 52 matching lines @@
 // static
 void SessionCrashedBubbleView::ShowForReal(
     std::unique_ptr<BrowserRemovalObserver> browser_observer,
     bool uma_opted_in_already) {
   // Determine whether or not the UMA opt-in option should be offered. It is
   // offered only when it is a Google chrome build, user hasn't opted in yet,
   // and the preference is modifiable by the user.
   bool offer_uma_optin = false;
 
 #if defined(GOOGLE_CHROME_BUILD)
-  if (!uma_opted_in_already) {
-    offer_uma_optin =
-        g_browser_process->local_state()
-            ->FindPreference(metrics::prefs::kMetricsReportingEnabled)
-            ->IsUserModifiable();
-  }
+  if (!uma_opted_in_already)
+    offer_uma_optin = !IsMetricsReportingPolicyManaged();

[msw, 2016/10/07 16:54:29]

q: just double checking, IsUserModifiable() == !IsManaged()?

[Alexei Svitkine (slow), 2016/10/07 17:33:19]

Yep, for metrics pref that is the case. Some prefs are connected to specific
command line flags and can be not modifiable due to those, but that is not the
case for metrics reporting pref. Note: We also use the same logic for the
options UI for this pref, so with this change the use of the API is consistent.

 #endif  // defined(GOOGLE_CHROME_BUILD)
 
   Browser* browser = browser_observer->browser();
 
   if (!browser) {
     RecordBubbleHistogramValue(SESSION_CRASHED_BUBBLE_ERROR);
     return;
   }
 
   views::View* anchor_view = BrowserView::GetBrowserViewForBrowser(browser)
@@ skipping 207 matching lines @@
   if (uma_option_ && uma_option_->checked()) {
     ChangeMetricsReportingState(true);
     RecordBubbleHistogramValue(SESSION_CRASHED_BUBBLE_UMA_OPTIN);
   }
   CloseBubble();
 }
 
 void SessionCrashedBubbleView::CloseBubble() {
   GetWidget()->Close();
 }