Add origin of settings reset request to feedback reports.

chrome/browser/resources/options/reset_profile_settings_overlay.js

 // Copyright 2013 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.
 
 cr.define('options', function() {
   var Page = cr.ui.pageManager.Page;
 
   var AutomaticSettingsResetBanner = options.AutomaticSettingsResetBanner;
 
   /**
    * ResetProfileSettingsOverlay class
    * Encapsulated handling of the 'Reset Profile Settings' overlay page.
    * @constructor
    * @extends {cr.ui.pageManager.Page}
    */
   function ResetProfileSettingsOverlay() {
     Page.call(this, 'resetProfileSettings',
               loadTimeData.getString('resetProfileSettingsOverlayTabTitle'),
               'reset-profile-settings-overlay');
   }
 
   cr.addSingletonGetter(ResetProfileSettingsOverlay);

[Dan Beam, 2016/09/13 18:42:09]

remove this addSingletonGetter() call.  it's not really needed and confusing you

change this code to use new ResetProfileSettingsOverlay()

https://cs.chromium.org/chromium/src/chrome/browser/resources/options/options...

[alito, 2016/09/16 02:10:37]

Done.

 
   ResetProfileSettingsOverlay.prototype = {
     // Inherit ResetProfileSettingsOverlay from Page.
     __proto__: Page.prototype,
 
+    /** @private {string} */

[Dan Beam, 2016/09/06 19:35:18]

this should be protected

[Dan Beam, 2016/09/13 18:42:09]

do this

[alito, 2016/09/16 02:10:37]

resetRequestOrigin_ is now a static variable.

+    resetRequestOrigin_: '',
+
     /** @override */
     initializePage: function() {
       Page.prototype.initializePage.call(this);
 
       $('reset-profile-settings-dismiss').onclick = function(e) {
         ResetProfileSettingsOverlay.dismiss();
       };
       $('reset-profile-settings-commit').onclick = function(e) {
         ResetProfileSettingsOverlay.setResettingState(true);
         chrome.send('performResetProfileSettings',
-                    [$('send-settings').checked]);
+                    [$('send-settings').checked,
+                     ResetProfileSettingsOverlay.getInstance()

[Dan Beam, 2016/09/06 19:35:18]

if ResetProfileSettingsOverlay.getInstance() != this, we shouldn't be storing
this variable in an instance.  it should be on a static instead, i.e.

ResetProfileSettingsOverlay.resetRequestOrigin_ =

[alito, 2016/09/13 02:02:00]

The reason that ResetProfileSettingsOverlay.getInstance() != this every time is
because initializePage() is not overridden by
TriggeredResetProfileSettingsOverlay. So initializePage() gets called twice:
once for ResetProfileSettingsOverlay and once for
TriggeredResetProfileSettingsOverlay. See my suggestion 3.c below.
So I would create a non-const public static variable? I don't see any uses of
non-const static variables in the options code base. Are you ok with that? My JS
is weak, but my understanding is that we should avoid these (correct me if I'm
wrong).

The way I see it, I have five options for how to store and access
resetRequestOrigin_:

1) Non-const public static variable
   The static variable would be initialized in the outermost function with
ResetProfileSettingsOverlay.resetRequestOrigin_ = ... and would be accessed
directly by both TriggeredResetProfileSettingsOverlay and the onclick function
here.

2) Local static variable (ie defined in the same scope as the variable 'Page' at
the top of the file)
   I could then define static setter/getter functions. The setter would be used
by TriggeredResetProfileSettingsOverlay to set resetRequestOrigin. The getter
would be used by the onclick function here (like
ResetProfileSettingsOverlay.getResetRequestOrigin()).

3) a private instance variable

3.a) accessed by calling getInstance()
     This works because ResetProfileSettingsOverlay is a singleton (so I guess
it is not that different from having a static variable?) and this is the
approach in this patch set.

3.b) accessed via static getter/setter functions
     The static getter/setter functions would call getInstance() to access
resetRequestOrigin_. An example of this usage:
https://cs.chromium.org/chromium/src/chrome/browser/resources/options/clear_b...

3.c) accessed via 'this'
     To make this work, I would override initializePage() in
TriggeredResetProfileSettingsOverlay and make that into essentially a no-op.
That way, the onclick handlers would only be set once (instead of twice, which
is the case now) and I could use 'this' to access resetRequestOrigin_.
   
Which approach do you recommend? Feel free to suggest a better alternative I
haven't considered.

[Dan Beam, 2016/09/13 18:42:09]

tl;dr

+                         .resetRequestOrigin_]);

[Dan Beam, 2016/09/06 19:35:18]

this.resetRequestOrigin_

[Dan Beam, 2016/09/13 18:42:09]

do this

[alito, 2016/09/16 02:10:37]

resetRequestOrigin_ is now a static variable.

       };

[Dan Beam, 2016/09/06 19:35:18]

}.bind(this);

[Dan Beam, 2016/09/13 18:42:09]

do this

[alito, 2016/09/16 02:10:37]

No longer needed.

       $('expand-feedback').onclick = function(e) {
         var feedbackTemplate = $('feedback-template');
         feedbackTemplate.hidden = !feedbackTemplate.hidden;
         e.preventDefault();
       };
     },
 
     /**
      * @override
      * @suppress {checkTypes}
      * TODO(vitalyp): remove the suppression. See the explanation in
      * chrome/browser/resources/options/automatic_settings_reset_banner.js.
      */
     didShowPage: function() {
+      // A '#userclick' hash indicates that the reset request came from the
+      // user clicking on the reset settings button. This hash is set by the
+      // browser_options page.
+      //
+      // A '#cct' hash indicates that the reset request came from the CCT by
+      // launching Chrome with the startup URL
+      // chrome://settings/resetProfileSettings#cct.
+      var hash = this.hash.slice(1).toLowerCase();
+      this.resetRequestOrigin_ =
+          hash == 'cct' || hash == 'userclick' ? hash : '';
+      this.setHash('');
+
       $('reset-profile-settings-title').textContent =
           loadTimeData.getString('resetProfileSettingsOverlay');
       $('reset-profile-settings-explanation').textContent =
           loadTimeData.getString('resetProfileSettingsExplanation');
 
       chrome.send('onShowResetProfileDialog');
     },
 
     /** @override */
     didClosePage: function() {
@@ skipping 36 matching lines @@
     var input = new JsEvalContext(feedbackListData);
     var output = $('feedback-template');
     jstProcess(input, output);
   };
 
   // Export
   return {
     ResetProfileSettingsOverlay: ResetProfileSettingsOverlay
   };
 });