[MD settings] on startup use current pages

chrome/browser/resources/settings/on_startup_page/startup_urls_page.js

Index: chrome/browser/resources/settings/on_startup_page/startup_urls_page.js
diff --git a/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js b/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js
index 9bce9e97b960c5ab00f652ffabac3385e336fb0d..ecf8e63c240646a0a355ea306325d53dd74945e4 100644
--- a/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js
+++ b/chrome/browser/resources/settings/on_startup_page/startup_urls_page.js
@@ -36,18 +36,50 @@ Polymer({
     /** @type {!Array<string>} */
     savedUrlList: {
       type: Array,
-      value: function() { return []; }
     },
   },
 
+  observers: [
+    'prefsChanged_(prefs.session.startup_urls.value.*)',
+  ],
+
+  attached: function() {
+    var caller = this;

[michaelpg, 2015/09/14 19:13:49]

use .bind instead of caching this

[dschuyler, 2015/09/14 23:20:00]

Done.

+    cr.define('Settings', function() {

[michaelpg, 2015/09/14 19:13:48]

What is this for?

[dschuyler, 2015/09/14 23:20:00]

This is routing the 'Settings.updateStartupPages' javascript call from c++ to
the local this.updateStartupPages_.

+      return {
+        updateStartupPages: function(data) {
+          caller.updateStartupPages_(data);
+        },
+      };
+    });
+  },
+
+  prefsChanged_: function(change) {
+    if (this.savedUrlList === undefined &&

[michaelpg, 2015/09/14 19:13:48]

use double equals (the property shouldn't be null, right?)

[dschuyler, 2015/09/14 23:20:00]

Actually, I'm expecting it to be undefined.  I don't expect it to be null.  So I
think that means === is the correct choice(?).

i.e. undefined === undefined (but undefined === null is false).
whereas with 
x == undefined then x may be undefined or null?

Or are you saying that we should allow savedUrlList to be updated, if somehow
(and unexpectedly) savedUrlList becomes null?

I'm changing it to == assuming that you're looking for null check
just-to-be-extra-safe.  I.e. it's not strictly required.

[michaelpg, 2015/09/17 21:18:13]

We rarely use strict equality (===) which is the main reason I mentioned it. If:

* there's no way for "this.savedUrlList" to be null, and
* if hypothetically "this.savedUrlList" WERE null, we'd want to do this action
anyway

then there's no reason to use === and then less specific == is fine.
basically
Just convention, not safety. It's only safer in the sense that if we decide to
initialize savedUrlList to null for some random reason instead of undefined, we
won't break this.

+        this.get('prefs.session.startup_urls')) {
+      this.savedUrlList = this.prefs.session.startup_urls.value.slice();
+    }
+  },
+
+  /** @private */
+  updateStartupPages_: function(data) {
+    var urlArray = [];
+    for (var i = 0; i < data.length; ++i) {

[michaelpg, 2015/09/14 19:13:48]

no braces around single line

[dschuyler, 2015/09/14 23:20:00]

Done.

+      urlArray.push(data[i].url);

[michaelpg, 2015/09/14 19:13:49]

or:
var urlArray = data.map(function(datum) { return datum.url; });

[dschuyler, 2015/09/14 23:20:00]

heh, was this an fyi and I can use either one - or a request to map instead of a
for loop?

I'm used to for-loops being remarkably faster than map(), but maybe that's
changed since I last checked?

Done.

[michaelpg, 2015/09/17 21:18:13]

"or" meant "optional" :-) I personally prefer this style when
you're essentially doing something as simple as
  x = [lambda(x) -> x.url].
But feel free to change it back.

I dunno if map is necessarily slower than a loop. I'm seeing
conflicting results here:
http://jsperf.com/aoeuaoeuaoeuaoeuueoueo

Sometimes the loops is faster, sometimes the map is
faster ¯\_(ツ)_/¯

+    }
+    this.set('prefs.session.startup_urls.value', urlArray);
+  },
+
   /** @private */
   onUseCurrentPagesTap_: function() {
-    // TODO(dschuyler): I'll be making a chrome.send call here.
+    chrome.send('setStartupPagesToCurrentPages');
   },
 
   /** @private */
   onCancelTap_: function() {
-    this.set('prefs.session.startup_urls.value', this.savedUrlList.slice());
+    if (this.savedUrlList !== undefined) {
+      this.set('prefs.session.startup_urls.value', this.savedUrlList.slice());
+    }
   },
 
   /** @private */