Stop checking for null selectionModel field.

chrome/browser/resources/options/language_list.js

 // Copyright (c) 2012 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() {
   /** @const */ var ArrayDataModel = cr.ui.ArrayDataModel;
   /** @const */ var DeletableItem = options.DeletableItem;
   /** @const */ var DeletableItemList = options.DeletableItemList;
   /** @const */ var List = cr.ui.List;
   /** @const */ var ListItem = cr.ui.ListItem;
@@ skipping 377 matching lines @@
      * @private
      */
     load_: function(languageCodes) {
       // Preserve the original selected index. See comments below.
       var originalSelectedIndex = (this.selectionModel ?
                                    this.selectionModel.selectedIndex : -1);
       this.dataModel = new ArrayDataModel(languageCodes);
       if (originalSelectedIndex >= 0 &&
           originalSelectedIndex < this.dataModel.length) {
         // Restore the original selected index if the selected index is
         // valid after the data model is loaded. This is neeeded to keep

[Dan Beam, 2016/01/25 23:27:16]

needed

[Kevin Bailey, 2016/01/26 15:00:40]

Done.

         // the selected language after the languge is added or removed.
         this.selectionModel.selectedIndex = originalSelectedIndex;
         // The lead index should be updated too.
         this.selectionModel.leadIndex = originalSelectedIndex;
-      } else if (this.dataModel.length > 0) {
-        // Otherwise, select the first item if it's not empty.
-        // Note that ListSingleSelectionModel won't select an item
-        // automatically, hence we manually select the first item here.

[Dan Beam, 2016/01/25 23:27:16]

this is why selectedIndex starts as -1:
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...
(which makes sense because the selection model is likely started without a 0th
item)

-        this.selectionModel.selectedIndex = 0;

[Kevin Bailey, 2016/01/22 15:12:50]

While looking around, trying to figure out how storage of language order
preference worked, I spotted this. It uses a null .selectionModel field on line
407.

I suspect that it's not a problem because .selectionModel is always set (if
::decorate() is always called) but I assume we still want to fix it.

It's also unclear to me how .selectionModel.selectedIndex would even change
during the new ArrayDataModel() call, but maybe the side effect of setting
.leadIndex is the real purpose (although this is strange too, since the
.selectedIndex setter takes care of .leadIndex.) Maybe the whole method can go
away?

If someone can confirm this understanding, I could clean it up more.

[please use gerrit instead, 2016/01/22 18:31:09]

I defer to Dan as the resident JS expert.

[Dan Beam, 2016/01/25 23:27:16]

this is where selectionModel is created (on page load an existing DOM node is
"decorated"):
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

you would only hit a null pointer exception here if load_ is called before the
page is ready (which seems unlikely but possible).
generally we should defer or drop actions if the item is not yet decorated. 
most of the code that would require the element to be decorated... runs at the
same time, though, which is why we don't typically hit issues.
this.dataModel = ... actually modifies this.selectionModel as a side effect (in
some cases):
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...
probably not
i can only confirm we don't have full understanding ;)

       }
     },
 
     /**
      * Saves the preference.
      */
     savePreference_: function() {
       chrome.send('updateLanguageList', [this.dataModel.slice()]);
       cr.dispatchSimpleEvent(this, 'save');
     },
@@ skipping 18 matching lines @@
       }
       return filteredLanguageCodes;
     },
   };
 
   return {
     LanguageList: LanguageList,
     LanguageListItem: LanguageListItem
   };
 });