Re-add backspace-goes-back as a default disabled finch trial

chrome/browser/ui/browser_command_controller.cc

Index: chrome/browser/ui/browser_command_controller.cc
diff --git a/chrome/browser/ui/browser_command_controller.cc b/chrome/browser/ui/browser_command_controller.cc
index 7f16d96dedcb1bc010bbc4dda67f732d17f2d209..9320f5a20243b05f67795d50987f1ebb5c30554d 100644
--- a/chrome/browser/ui/browser_command_controller.cc
+++ b/chrome/browser/ui/browser_command_controller.cc
@@ -11,6 +11,7 @@
 #include "base/command_line.h"
 #include "base/debug/debugging_flags.h"
 #include "base/debug/profiler.h"
+#include "base/feature_list.h"
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
@@ -117,6 +118,10 @@ bool HasInternalURL(const NavigationEntry* entry) {
 
 namespace chrome {
 
+const base::Feature kBackspaceGoesBackFeature {
+  "BackspaceGoesBack", base::FEATURE_DISABLED_BY_DEFAULT
+};
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserCommandController, public:
 
@@ -315,6 +320,14 @@ void BrowserCommandController::ExecuteCommandWithDisposition(
   // declaration order in browser.h!
   switch (id) {
     // Navigation commands
+    case IDC_BACKSPACE_BACK:
+      if (base::FeatureList::IsEnabled(kBackspaceGoesBackFeature))
+        GoBack(browser_, disposition);
+      break;
+    case IDC_BACKSPACE_FORWARD:
+      if (base::FeatureList::IsEnabled(kBackspaceGoesBackFeature))
+        GoForward(browser_, disposition);
+      break;
     case IDC_BACK:
       GoBack(browser_, disposition);
       break;

[Peter Kasting, 2016/05/19 09:28:30]

Nit: Slightly shorter, and abides by the comment about keeping the command order
exactly matching the function declaration order:

    case IDC_BACKSPACE_BACK:
      if (!base::FeatureList::IsEnabled(kBackspaceGoesBackFeature))
        break;
    case IDC_BACK:
      GoBack(browser_, disposition);
      break;

(& similar)

[ojan, 2016/05/20 16:33:48]

done

[Matt Giuca, 2016/05/23 01:12:01]

Um, this is kind of gross and I'm going to have to put it back the way it was in
my follow-up CL (which adds an else clause)... I have never seen a conditional
fall-through before.

[Matt Giuca, 2016/05/23 01:15:40]

Sorry, actually I can keep the conditional fall-through if it's desired. Seems
bad though. Will discuss on my follow-up CL
https://codereview.chromium.org/1983803002/

@@ -978,6 +991,10 @@ void BrowserCommandController::UpdateCommandsForTabState() {
     return;
 
   // Navigation commands
+  command_updater_.UpdateCommandEnabled(IDC_BACKSPACE_BACK,
+                                        CanGoBack(browser_));
+  command_updater_.UpdateCommandEnabled(IDC_BACKSPACE_FORWARD,

[Peter Kasting, 2016/05/19 09:28:30]

Nit: Swap this with next line to keep the backs/forwards together

[ojan, 2016/05/20 16:33:48]

done

+                                        CanGoForward(browser_));
   command_updater_.UpdateCommandEnabled(IDC_BACK, CanGoBack(browser_));
   command_updater_.UpdateCommandEnabled(IDC_FORWARD, CanGoForward(browser_));
   command_updater_.UpdateCommandEnabled(IDC_RELOAD, CanReload(browser_));