Better session restore: UI modifications.

Only show the info dialog if the user has the "clear data on exit" setting
or the session only cookie setting selected.

BUG=116046
TEST=Manual, see bug.


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

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

5 files changed, 105 insertions, 23 deletions

diff --git a/chrome/browser/resources/options2/browser_options.css b/chrome/browser/resources/options2/browser_options.css
index b76299c..b446637 100644
--- a/chrome/browser/resources/options2/browser_options.css
+++ b/chrome/browser/resources/options2/browser_options.css
@@ -336,3 +336,7 @@ input[type='range'] {
 #advanced-settings.sliding {
   -webkit-transition: height 200ms;
 }
+
+#sessionRestoreOverlay {
+  width: 500px;
+}
diff --git a/chrome/browser/resources/options2/browser_options.html b/chrome/browser/resources/options2/browser_options.html
index bc3aee5..fb950ea 100644
--- a/chrome/browser/resources/options2/browser_options.html
+++ b/chrome/browser/resources/options2/browser_options.html
@@ -60,8 +60,8 @@
       </div>
       <div class="radio">
         <label>
-          <input type="radio" name="startup" value="1"
-              pref="session.restore_on_startup"
+          <input id="startup-restore-session" type="radio" name="startup"
+              value="1" pref="session.restore_on_startup"
               metric="Options_Startup_LastSession">
           <span id="old-startup-last-text"
               i18n-content="startupShowLastSession">
@@ -559,8 +559,8 @@
       </div>
       <div class="radio">
         <label>
-          <input type="radio" name="startup" value="1"
-              pref="session.restore_on_startup"
+          <input id="startup-restore-session" type="radio" name="startup"
+              value="1" pref="session.restore_on_startup"
               metric="Options_Startup_LastSession">
           <span id="old-startup-last-text"
               i18n-content="startupShowLastSession">
diff --git a/chrome/browser/resources/options2/browser_options.js b/chrome/browser/resources/options2/browser_options.js
index 4e9d3c6..6119444 100644
--- a/chrome/browser/resources/options2/browser_options.js
+++ b/chrome/browser/resources/options2/browser_options.js
@@ -35,6 +35,24 @@ cr.define('options', function() {
     instantConfirmDialogShown_: false,
 
     /**
+     * True if the default cookie settings is session only. Used for deciding
+     * whether to show the session restore info dialog. The value is undefined
+     * until the preference has been read.
+     * @type {bool}
+     * @private
+     */
+    sessionOnlyCookies_: undefined,
+
+    /**
+     * True if the "clear cookies and other site data on exit" setting is
+     * selected. Used for deciding whether to show the session restore info
+     * dialog. The value is undefined until the preference has been read.
+     * @type {bool}
+     * @private
+     */
+    clearCookiesOnExit_: undefined,
+
+    /**
      * @inheritDoc
      */
     initializePage: function() {
@@ -83,10 +101,17 @@ cr.define('options', function() {
         OptionsPage.navigateToPage('startup');
       };
 
+      // Session restore.
       this.sessionRestoreEnabled_ = templateData.enable_restore_session_state;
       if (this.sessionRestoreEnabled_) {
         $('old-startup-last-text').hidden = true;
         $('new-startup-last-text').hidden = false;
+        $('startup-restore-session').customChangeHandler = function(event) {
+          if (this.checked)
+            BrowserOptions.getInstance().maybeShowSessionRestoreDialog_();
+          // Continue the normal event handling (set the preference).
+          return false;
+        };
       }
 
       // Appearance section.
@@ -146,11 +171,16 @@ cr.define('options', function() {
       Preferences.getInstance().addEventListener('instant.enabled',
           this.onInstantEnabledChanged_.bind(this));
       Preferences.getInstance().addEventListener(
-          'session.restore_on_startup',
-          this.onSessionRestoreSelectedChanged_.bind(this));
-      Preferences.getInstance().addEventListener(
           'restore_session_state.dialog_shown',
           this.onSessionRestoreDialogShownChanged_.bind(this));
+      var self = this;
+      Preferences.getInstance().addEventListener(
+          'profile.clear_site_data_on_exit',
+          function(event) {
+            if (event.value && typeof event.value['value'] != 'undefined') {
+              self.clearCookiesOnExit_ = event.value['value'] == true;
+            }
+          });
 
       // Users section.
       var profilesList = $('profiles-list');
@@ -702,24 +732,30 @@ cr.define('options', function() {
                                              'instant-enabled-control';
     },
 
-    onSessionRestoreSelectedChanged_: function(event) {
-      this.sessionRestoreSelected_ = event.value['value'] == 1;
-      this.maybeShowSessionRestoreDialog_();
-    },
-
+    /**
+     * Called when the value of the restore_session_state.dialog_shown
+     * preference changes.
+     * @param {Event} event Change event.
+     * @private
+     */
     onSessionRestoreDialogShownChanged_: function(event) {
       this.sessionRestoreDialogShown_ = event.value['value'];
-      this.maybeShowSessionRestoreDialog_();
     },
 
+    /**
+     * Displays the session restore info dialog if options depending on sessions
+     * (session only cookies or clearning data on exit) are selected, and the
+     * dialog has never been shown.
+     * @private
+     */
     maybeShowSessionRestoreDialog_: function() {
       // Don't show this dialog in Guest mode.
       if (cr.isChromeOS && AccountsOptions.loggedInAsGuest())
         return;
-      // If either of the needed two preferences hasn't been read yet, the
+      // If some of the needed preferences haven't been read yet, the
       // corresponding member variable will be undefined and we won't display
       // the dialog yet.
-      if (this.sessionRestoreEnabled_ && this.sessionRestoreSelected_ &&
+      if (this.userHasSelectedSessionContentSettings_() &&
           this.sessionRestoreDialogShown_ === false) {
         this.sessionRestoreDialogShown_ = true;
         Preferences.setBooleanPref('restore_session_state.dialog_shown', true);
@@ -728,6 +764,16 @@ cr.define('options', function() {
     },
 
     /**
+     * Returns true if the user has selected content settings which rely on
+     * sessions: clearning the browsing data on exit or defaulting the cookie
+     * content setting to session only.
+     * @private
+     */
+    userHasSelectedSessionContentSettings_: function() {
+      return this.clearCookiesOnExit_ || this.sessionOnlyCookies_;
+    },
+
+    /**
      * Update the Default Browsers section based on the current state.
      * @param {string} statusString Description of the current default state.
      * @param {boolean} isDefault Whether or not the browser is currently
@@ -1168,7 +1214,19 @@ cr.define('options', function() {
         if (index != undefined)
           $('bluetooth-paired-devices-list').deleteItemAtIndex(index);
       }
+    },
+
+    /**
+     * Called when the default content setting value for a content type changes.
+     * @param {Object} dict A mapping content setting types to the default
+     * value.
+     * @private
+     */
+    setContentFilterSettingsValue_: function(dict) {
+      if ('cookies' in dict && 'value' in dict['cookies'])
+        this.sessionOnlyCookies_ = dict['cookies']['value'] == 'session';
     }
+
   };
 
   //Forward public APIs to private implementations.
@@ -1183,6 +1241,7 @@ cr.define('options', function() {
     'setBackgroundModeCheckboxState',
     'setBluetoothState',
     'setCheckRevocationCheckboxState',
+    'setContentFilterSettingsValue',
     'setFontSize',
     'setGtkThemeButtonEnabled',
     'setHighContrastCheckboxState',
diff --git a/chrome/browser/resources/options2/content_settings.js b/chrome/browser/resources/options2/content_settings.js
index b96d646..89cf41b 100644
--- a/chrome/browser/resources/options2/content_settings.js
+++ b/chrome/browser/resources/options2/content_settings.js
@@ -20,13 +20,14 @@ cr.define('options', function() {
                      'content-settings-page');
 
     // Keep track of the real value of the "clear on exit" preference. (The UI
-    // might override it if the reopen last pages setting is selected.)
+    // might override it if the "continue where I left off" setting is
+    // selected.)
     var self = this;
     Preferences.getInstance().addEventListener(
         'profile.clear_site_data_on_exit',
         function(event) {
-          if (event.value && event.value['value'] != undefined) {
-            self.clearCookiesOnExit = (event.value['value'] == true);
+          if (event.value && typeof event.value['value'] != 'undefined') {
+            self.clearCookiesOnExit = event.value['value'] == true;
           }
         });
     Preferences.getInstance().addEventListener(
@@ -91,11 +92,16 @@ cr.define('options', function() {
           OptionsPage.closeOverlay.bind(OptionsPage);
     },
 
+    /**
+     * Called when the value of the "On startup" setting changes.
+     * @param {Event} event Change event.
+     * @private
+     */
     onSessionRestoreSelectedChanged: function(event) {
-      if (!event.value || event.value['value'] == undefined)
+      if (!event.value || typeof event.value['value'] == 'undefined')
         return;
 
-      this.sessionRestoreSelected = (event.value['value'] == 1);
+      this.sessionRestoreSelected = event.value['value'] == 1;
 
       if (this.sessionRestoreSelected)
         this.updateSessionRestoreContentSettings();
@@ -103,8 +109,8 @@ cr.define('options', function() {
         this.restoreContentSettings();
     },
 
-    // If the "reopen last opened pages" setting is selected, disable the "clear
-    // on exit" checkbox, and the "session only" setting for cookies.
+    // If the "continue where I left off" setting is selected, disable the
+    // "clear on exit" checkbox, and the "session only" setting for cookies.
     updateSessionRestoreContentSettings: function() {
       // This feature is behind a command line flag.
       if (this.sessionRestoreEnabled && this.sessionRestoreSelected) {
@@ -159,7 +165,7 @@ cr.define('options', function() {
     // if the reopen last pages setting is selected.)
     if ('cookies' in dict && 'value' in dict['cookies']) {
       ContentSettings.getInstance().cookiesSession =
-          (dict['cookies']['value'] == 'session');
+          dict['cookies']['value'] == 'session';
     }
     ContentSettings.getInstance().updateSessionRestoreContentSettings();
     OptionsPage.updateManagedBannerVisibility();
diff --git a/chrome/browser/resources/options2/pref_ui.js b/chrome/browser/resources/options2/pref_ui.js
index e4179d5..e255ce0 100644
--- a/chrome/browser/resources/options2/pref_ui.js
+++ b/chrome/browser/resources/options2/pref_ui.js
@@ -192,6 +192,8 @@ cr.define('options', function() {
       // Listen to preference changes.
       Preferences.getInstance().addEventListener(this.pref,
           function(event) {
+            if (self.customChangeHandler(event))
+              return;
             var value = event.value && event.value['value'] != undefined ?
                 event.value['value'] : event.value;
             self.checked = String(value) == self.value;
@@ -235,6 +237,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;
+    },
   };
 
   /**