Settings People Revamp: Simplify Change Picture browsertests.

chrome/browser/resources/settings/people_page/change_picture.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.
 
 cr.exportPath('settings_test');
 
 /** @type {boolean} */
 settings_test.changePictureNotifyForTest;
 
 /**
@@ skipping 29 matching lines @@
     cameraPresent_: {
       type: Boolean,
       value: false,
     },
 
     /**
      * The currently selected item. This property is bound to the iron-selector
      * and never directly assigned.
      * @private {settings.ChangePictureImageElement}
      */
     selectedItem_: {

[dpapad, 2016/04/09 00:16:35]

Shouldn't this be

 /**
  * @private {settings.ChangePictureImageElement}
  */
 selectedItem_: Object,


I don't think you can use the typedef name as the "type" value in a Polymer
property, only Boolean, Date, Number, String, Array or Object is accepted.

[tommycli, 2016/04/11 19:09:57]

Done. Yes I think you are correct. I wonder what is actually happening when I do
this.

       type: settings.ChangePictureImageElement,
-      notify: !!settings_test.changePictureNotifyForTest,
+      value: null,
     },
 
     /**
      * The url of the 'old' image, which is the existing image sourced from
      * the camera, a file, or a deprecated default image. It defaults to an
      * empty string instead of undefined, because Polymer bindings don't behave
      * as expected with undefined properties.
      * @private {string}
      */
     oldImageUrl_: {
@@ skipping 18 matching lines @@
       type: Array,
       value: function() { return []; },
     },
 
     /**
      * The fallback image to be selected when the user discards the 'old' image.
      * This may be null if the user started with the 'old' image.
      * @private {?settings.ChangePictureImageElement}
      */
     fallbackImage_: {
       type: settings.ChangePictureImageElement,

[dpapad, 2016/04/09 00:16:35]

Same here.

[tommycli, 2016/04/11 19:09:57]

Done.

       value: null,
     },
 
     /**
      * Type of the last selected icon. This is used to jump back to the camera
      * after the user discards a newly taken photo.
      * @private {string}
      */
     lastSelectedImageType_: {
       type: String,
@@ skipping 213 matching lines @@
     if (this.fallbackImage_ != null) {
       this.selectImage_(this.fallbackImage_);
       return;
     }
 
     // If the user has not chosen an image since opening the subpage and
     // discards the current photo, select the first default image.
     assert(this.defaultImages_.length > 0);
     this.browserProxy_.selectDefaultImage(this.defaultImages_[0].url);
 
-    announceAccessibleMessage(
-        loadTimeData.getString('photoDiscardAccessibleText'));
+    announceAccessibleMessage(this.i18n('photoDiscardAccessibleText'));
   },
 
   /**
    * @param {string} oldImageUrl
    * @return {boolean} True if there is no old image and the old image icon
    *     should be hidden.
    * @private
    */
   isOldImageHidden_: function(oldImageUrl) { return oldImageUrl.length == 0; },
 
   /**
    * @param {settings.ChangePictureImageElement} selectedItem
    * @return {boolean} True if the preview image should be hidden.
    * @private
    */
   isPreviewImageHidden_: function(selectedItem) {
     if (!selectedItem)
       return true;
 
     var type = selectedItem.dataset.type;
     return type != 'default' && type != 'profile' && type != 'old';
   },
 
   /**
    * @param {settings.ChangePictureImageElement} selectedItem
    * @return {boolean} True if the camera is selected in the image grid.
    * @private
    */
   isCameraActive_: function(cameraPresent, selectedItem) {
-    return cameraPresent && selectedItem &&
+    return cameraPresent && selectedItem != null &&

[dpapad, 2016/04/09 00:16:35]

Nit (optional): Use "!==" instead of "!=".

[tommycli, 2016/04/11 19:09:57]

This didn't work for me because even with the value: null, the value is
momentarily undefined when the page is first loaded. Not sure why.

[dpapad, 2016/04/11 19:36:38]

That is odd. Polymer data bindings guarantee that this function will not be
called until all dependencies (cameraPresent and selectedItem) are set to a
non-undefined value, pasting below from the docs:

"The computing function is not called until all dependent properties are defined
(!=undefined)."

I am fine with leaving as is, but it would be nice to understand why does the
behavior not match the Polymer docs. I have definitely seen the advertized
behavior working in other cases.

[tommycli, 2016/04/11 21:20:52]

I investigated this in detail. Basically: when iron-selector selects a different
item, the selectedItem momentarily becomes undefined, and then it becomes
defined again (to the new item).

I filed an issue: https://github.com/PolymerElements/iron-selector/issues/118

In the meantime i will leave these computed bindings as 'falsy'.

[dpapad, 2016/04/11 21:29:33]

Thanks for investigating!

         selectedItem.dataset.type == 'camera';
   },
 
   /**
    * @param {settings.ChangePictureImageElement} selectedItem
    * @return {boolean} True if the discard controls should be hidden.
    * @private
    */
   isDiscardHidden_: function(selectedItem) {
     return !selectedItem || selectedItem.dataset.type != 'old';
   },
 
   /**
    * @param {settings.ChangePictureImageElement} selectedItem
    * @return {boolean} True if the author credit text is shown.
    * @private
    */
   isAuthorCreditShown_: function(selectedItem) {
-    return selectedItem && selectedItem.dataset.type == 'default';
+    return selectedItem != null && selectedItem.dataset.type == 'default';
   },
 
   /**
    * @param {!settings.ChangePictureImageElement} selectedItem
    * @param {!Array<!settings.DefaultImage>} defaultImages
    * @return {string} The author name for the selected default image. An empty
    *     string is returned if there is no valid author name.
    * @private
    */
   getAuthorName_: function(selectedItem, defaultImages) {
@@ skipping 14 matching lines @@
    */
   getAuthorWebsite_: function(selectedItem, defaultImages) {
     if (!this.isAuthorCreditShown_(selectedItem))
       return '';
 
     assert(selectedItem.dataset.defaultImageIndex !== null &&
            selectedItem.dataset.defaultImageIndex < defaultImages.length);
     return defaultImages[selectedItem.dataset.defaultImageIndex].website;
   },
 });