[MD settings] on startup use current pages

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

 // Copyright 2015 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.
 
 /**
  * @fileoverview 'cr-settings-startup-urls-page' is the settings page
  * containing the urls that will be opened when chrome is started.
  *
  * Example:
  *
@@ skipping 18 matching lines @@
       notify: true,
     },
 
     newUrl: {
       type: String,
     },
 
     /** @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 */
   onOkTap_: function() {
     var value = this.newUrl && this.newUrl.trim();
     if (!value)
       return;
     this.push('prefs.session.startup_urls.value', value);
     this.newUrl = undefined;
   },
 
   /** @private */
   onRemoveUrlTap_: function(e) {
     this.splice('prefs.session.startup_urls.value', e.model.index, 1);
   },
 });