Refactor Instant web UI (chrome://settings page).

Currently, the web UI directly sets two Instant prefs (instant.enabled and
instant.confirm_dialog_shown), but doesn't call InstantController's Enable()
or Disable(). Those methods set some additional prefs, so this refactoring
calls those instead. This will also be helpful in a later CL where we enable
Instant by default in a field trial.

There are two behavioural changes:
+ The kInstantEnabledOnce pref is set, even for a disable (@sky approves).
+ The first time the user enables Instant, the old behaviour used to be that
  the checkbox would get cleared, then the confirm dialog would be shown. I
  found that this was slightly janky, so we now leave the checkbox enabled
  when showing the confirm dialog. After the user clicks OK or Cancel, the
  checkbox acquires the expected state (as before).

BUG=none
TEST=Enabled/disabled checkbox, and saw no regression in behaviour.


Review URL: http://codereview.chromium.org/7396025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@94005 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 46 insertions, 11 deletions

diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc
index c25f62c..4e99b3d 100644
--- a/chrome/browser/instant/instant_controller.cc
+++ b/chrome/browser/instant/instant_controller.cc
@@ -135,6 +135,7 @@ void InstantController::Disable(Profile* profile) {
                                 delta.InMinutes(), 1, 60 * 24 * 10, 50);
   }
 
+  service->SetBoolean(prefs::kInstantEnabledOnce, true);
   service->SetBoolean(prefs::kInstantEnabled, false);
 }
 
diff --git a/chrome/browser/resources/options/browser_options.html b/chrome/browser/resources/options/browser_options.html
index 6aaa0c5..d03df9b 100644
--- a/chrome/browser/resources/options/browser_options.html
+++ b/chrome/browser/resources/options/browser_options.html
@@ -94,7 +94,7 @@
           <div id="instantOption" class="checkbox">
             <label>
               <!-- TODO(estade): metric? -->
-              <input id="instantEnableCheckbox" type="checkbox"
+              <input id="instantEnabledCheckbox" type="checkbox"
                   pref="instant.enabled">
               <span i18n-content="instantName"></span>
             </label>
diff --git a/chrome/browser/resources/options/browser_options.js b/chrome/browser/resources/options/browser_options.js
index 9a04314..ffd838e 100644
--- a/chrome/browser/resources/options/browser_options.js
+++ b/chrome/browser/resources/options/browser_options.js
@@ -71,13 +71,16 @@ cr.define('options', function() {
       $('defaultSearchEngine').onchange = this.setDefaultSearchEngine_;
 
       var self = this;
-      $('instantEnableCheckbox').onclick = function(event) {
-        if (this.checked && !self.instantConfirmDialogShown_) {
-          // Leave disabled for now. The PrefCheckbox handler already set it to
-          // true so undo that.
-          Preferences.setBooleanPref(this.pref, false, this.metric);
-          OptionsPage.navigateToPage('instantConfirm');
+      $('instantEnabledCheckbox').customChangeHandler = function(event) {
+        if (this.checked) {
+          if (self.instantConfirmDialogShown_)
+            chrome.send('enableInstant');
+          else
+            OptionsPage.navigateToPage('instantConfirm');
+        } else {
+          chrome.send('disableInstant');
         }
+        return true;
       };
 
       Preferences.getInstance().addEventListener('instant.confirm_dialog_shown',
diff --git a/chrome/browser/resources/options/instant_confirm_overlay.js b/chrome/browser/resources/options/instant_confirm_overlay.js
index 9f983c7..01a9ee5 100644
--- a/chrome/browser/resources/options/instant_confirm_overlay.js
+++ b/chrome/browser/resources/options/instant_confirm_overlay.js
@@ -22,13 +22,12 @@ cr.define('options', function() {
 
       $('instantConfirmCancel').onclick = function() {
         OptionsPage.closeOverlay();
+        $('instantEnabledCheckbox').checked = false;
       };
+
       $('instantConfirmOk').onclick = function() {
         OptionsPage.closeOverlay();
-        Preferences.setBooleanPref('instant.confirm_dialog_shown', true);
-        var instantEnabledCheckbox = $('instantEnableCheckbox');
-        Preferences.setBooleanPref(instantEnableCheckbox.pref, true,
-                                   instantEnableCheckbox.metric);
+        chrome.send('enableInstant');
       };
     },
   };
diff --git a/chrome/browser/resources/options/pref_ui.js b/chrome/browser/resources/options/pref_ui.js
index 16af4d0..2d88886 100644
--- a/chrome/browser/resources/options/pref_ui.js
+++ b/chrome/browser/resources/options/pref_ui.js
@@ -98,6 +98,8 @@ cr.define('options', function() {
       this.addEventListener(
           'change',
           function(e) {
+            if (self.customChangeHandler(e))
+              return;
             var value = self.inverted_pref ? !self.checked : self.checked;
             switch(self.valueType) {
               case 'number':
@@ -126,6 +128,17 @@ cr.define('options', function() {
     setDisabled: function(reason, disabled) {
       updateDisabledState_(this, reason, disabled);
     },
+
+    /**
+     * This method is called first while processing an onchange event. If it
+     * returns false, regular onchange processing continues (setting the
+     * associated pref, etc). If it returns true, the rest of the onchange is
+     * not performed. I.e., this works like stopPropagation or cancelBubble.
+     * @param {Event} event Change event.
+     */
+    customChangeHandler: function(event) {
+      return false;
+    },
   };
 
   /**
diff --git a/chrome/browser/ui/webui/options/browser_options_handler.cc b/chrome/browser/ui/webui/options/browser_options_handler.cc
index 3af3994..622b6e01 100644
--- a/chrome/browser/ui/webui/options/browser_options_handler.cc
+++ b/chrome/browser/ui/webui/options/browser_options_handler.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/custom_home_pages_table_model.h"
 #include "chrome/browser/instant/instant_confirm_dialog.h"
+#include "chrome/browser/instant/instant_controller.h"
 #include "chrome/browser/net/url_fixer_upper.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
@@ -122,6 +123,12 @@ void BrowserOptionsHandler::RegisterMessages() {
   web_ui_->RegisterMessageCallback(
       "toggleShowBookmarksBar",
       NewCallback(this, &BrowserOptionsHandler::ToggleShowBookmarksBar));
+  web_ui_->RegisterMessageCallback(
+      "enableInstant",
+      NewCallback(this, &BrowserOptionsHandler::EnableInstant));
+  web_ui_->RegisterMessageCallback(
+      "disableInstant",
+      NewCallback(this, &BrowserOptionsHandler::DisableInstant));
 }
 
 void BrowserOptionsHandler::Initialize() {
@@ -469,6 +476,14 @@ void BrowserOptionsHandler::ToggleShowBookmarksBar(const ListValue* args) {
       NotificationService::NoDetails());
 }
 
+void BrowserOptionsHandler::EnableInstant(const ListValue* args) {
+  InstantController::Enable(web_ui_->GetProfile());
+}
+
+void BrowserOptionsHandler::DisableInstant(const ListValue* args) {
+  InstantController::Disable(web_ui_->GetProfile());
+}
+
 void BrowserOptionsHandler::OnResultChanged(bool default_match_changed) {
   const AutocompleteResult& result = autocomplete_controller_->result();
   ListValue suggestions;
diff --git a/chrome/browser/ui/webui/options/browser_options_handler.h b/chrome/browser/ui/webui/options/browser_options_handler.h
index 097ee00..e400130 100644
--- a/chrome/browser/ui/webui/options/browser_options_handler.h
+++ b/chrome/browser/ui/webui/options/browser_options_handler.h
@@ -90,6 +90,10 @@ class BrowserOptionsHandler : public OptionsPageUIHandler,
   // Notifies any listeners interested in this event.  |args| is ignored.
   void ToggleShowBookmarksBar(const ListValue* args);
 
+  // Enables/disables Instant.
+  void EnableInstant(const ListValue* args);
+  void DisableInstant(const ListValue* args);
+
   // Returns the string ID for the given default browser state.
   int StatusStringIdForState(ShellIntegration::DefaultWebClientState state);