Add the extended reporting checkbox to the malware interstitial v3

chrome/browser/resources/safe_browsing/safe_browsing_v3.js

 // 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.
 
 // Must match the commands handled by SafeBrowsingBlockingPage::CommandReceived.
 var SB_CMD_DO_REPORT = 'doReport';
 var SB_CMD_DONT_REPORT = 'dontReport';
 var SB_CMD_EXPANDED_SEE_MORE = 'expandedSeeMore';
 var SB_CMD_LEARN_MORE_2 = 'learnMore2';
 var SB_CMD_PROCEED = 'proceed';
 var SB_CMD_REPORT_ERROR = 'reportError';
 var SB_CMD_SHOW_DIAGNOSTIC = 'showDiagnostic';
 var SB_CMD_SHOW_PRIVACY = 'showPrivacy';
 var SB_CMD_TAKE_ME_BACK = 'takeMeBack';
 
 // Other constants defined in safe_browsing_blocking_page.cc.
 var SB_BOX_CHECKED = 'boxchecked';
 var SB_DISPLAY_CHECK_BOX = 'displaycheckbox';
+
+// This sets up the Extended Safe Browsing Reporting opt-in.
+function setupCheckbox() {
+  if (loadTimeData.getBoolean('ssl')) return;
+  if (loadTimeData.getBoolean('phishing')) return;

[Dan Beam, 2014/06/16 19:05:50]

^ are 'ssl' or 'phishing' defined in a .cc file somewhere like SB_*?

[mattm, 2014/06/16 21:02:21]

I wonder if it should have some single value like getString('pageType') ==
'malware' instead of a bunch of random bools like this.

[felt, 2014/06/17 05:41:54]

No. What I want to do down the line is create a parent class
SecurityInterstitial that contains common state, like these booleans and other
shared setup. Then have the SafeBrowsingBlockingPage and SSLBlockingPage hang
off of it to do the customized portions. That will make this neater.

+  if (!loadTimeData.getBoolean(SB_DISPLAY_CHECK_BOX)) return;

[Dan Beam, 2014/06/16 19:05:50]

nit: combine if -> returns

[felt, 2014/06/17 05:41:54]

Done.

[Dan Beam, 2014/06/17 17:52:54]

not done, but it was a nit

[felt, 2014/06/17 18:03:27]

Done for realsies.

+
+  $('optin-label').innerHTML = loadTimeData.getString('optinText');

[Dan Beam, 2014/06/16 19:05:50]

can you use .innerText or .textContent instead of .innerHTML?

[felt, 2014/06/17 05:41:54]

No, the string has a link in it.

+  $('optin-checkbox').checked = loadTimeData.getBoolean(SB_BOX_CHECKED);
+  $('malware-optin').classList.remove('hidden');

[mattm, 2014/06/16 21:02:21]

Should be possible to do a lot of this without JS, see the old html:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
Or was it done this way to avoid needing to initialize the loadTimeData for
these things to empty values for the other page types?

[felt, 2014/06/17 05:41:54]

Yes, I didn't want to have to set them to empty values for SSL. In a future
world where the SSL and Safe Browsing warnings share methods and constants, it
will be possible to just stick these in because they will have default empty
values.

+
+  $('optin-checkbox').addEventListener('click', function() {
+    if ($('optin-checkbox').checked)
+      sendCommand(SB_CMD_DO_REPORT);
+    else
+      sendCommand(SB_CMD_DONT_REPORT);

[Dan Beam, 2014/06/16 19:05:50]

ternary, imo

[felt, 2014/06/17 05:41:54]

Done.

+  });
+}