Settings People Revamp: Implement parts of ChromeOS Change Picture

chrome/browser/resources/settings/people_page/change_picture_private_api.js

Index: chrome/browser/resources/settings/people_page/change_picture_private_api.js
diff --git a/chrome/browser/resources/settings/people_page/change_picture_private_api.js b/chrome/browser/resources/settings/people_page/change_picture_private_api.js
new file mode 100644
index 0000000000000000000000000000000000000000..471c2c8b10937debac04cc0fb9ee1a57d484b7a9
--- /dev/null
+++ b/chrome/browser/resources/settings/people_page/change_picture_private_api.js
@@ -0,0 +1,144 @@
+// 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');
+
+/**
+ * An object describing a default image.
+ * @typedef {{
+ *   author: string,
+ *   title: string,
+ *   url: string,
+ *   website: string
+ * }}
+ */
+settings.DefaultImage;
+
+cr.define('settings', function() {
+  /**
+   * API which encapsulates messaging between JS and C++ for the ChromeOS
+   * Change Picture subpage.
+   * @constructor
+   */
+  function ChangePicturePrivateApi() {}

[dpapad, 2016/01/11 19:25:50]

How about instantiating an object from this class, instead of making everything
static? I think this would make the code less verbose and more readable. For
example

/**
 * @constructor
 * @param ...
 * @param ...
 */
function ChangePicturePrivateApi(
    getDefaultImagesCallback,
    getSelectedImageUrlCallback,
    getOldImageUrlCallback,
    getProfileImageUrlCallback) {
  this.getDefaultImagesCallback_ = getDefaultImagesCallback;
  this.getSelectedImageUrlCallback_ = getSelectedImageUrlCallback;
  this.getOldImageUrlCallback_ = getOldImageUrlCallback;
  this.getProfileImageUrlCallback_ = getProfileImageUrlCallback;

  // No need for the static initialize() method anymore.
  chrome.send('onChangePicturePageInitialized')
}

ChangePicturePrivateApi.prototype.receiveDefaultImages = function() {
  // Maybe the if check is not even needed anymore, because constructor
guarantees
  // that getDefaultImagesCallback_ has a value.
  if (this.getDefaultImagesCallback_)
    this.getDefaultImagesCallback_(images);
};

...
...


Then from your client code (change_picture.js), you simply have to call the
constructor and create an instance, instead of using the static
ChangePicturePrivateApi.initialize method. Also to expose the methods of
ChangePicturePrivateApi, such that they can be called from C++, you can do same
trick as with
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res....

This is just as suggestion (therefore optional), because I think less static
stuff makes the code more readable, but I understand how current implementation
works.

[tommycli, 2016/01/13 18:29:11]

Done. Based on your examples, I was able to eliminate ~80 lines of code!

+
+  /**
+   * URLs of special button images.
+   * @enum {string}
+   */
+  ChangePicturePrivateApi.ButtonImages = {
+    TAKE_PHOTO: 'chrome://theme/IDR_BUTTON_USER_IMAGE_TAKE_PHOTO',
+    CHOOSE_FILE: 'chrome://theme/IDR_BUTTON_USER_IMAGE_CHOOSE_FILE',
+    PROFILE_PICTURE: 'chrome://theme/IDR_PROFILE_PICTURE_LOADING'
+  };
+
+  /** @private {?function(!Array<settings.DefaultImage>)} */
+  ChangePicturePrivateApi.getDefaultImagesCallback_ = null;
+
+  /** @private {?function(string)} */
+  ChangePicturePrivateApi.getSelectedImageUrlCallback_ = null;
+
+  /** @private {?function(string)} */
+  ChangePicturePrivateApi.getOldImageUrlCallback_ = null;
+
+  /** @private {?function(string, boolean)} */
+  ChangePicturePrivateApi.getProfileImageUrlCallback_ = null;
+
+  /**
+   * Called from JavaScript. Initializes page by setting callbacks and
+   * retrieving the initial set of default images, profile image, etc.
+   * @param {!function(!Array<settings.DefaultImage>)} getDefaultImagesCallback
+   * @param {!function(string)} getSelectedImageUrlCallback
+   * @param {!function(string)} getOldImageUrlCallback
+   * @param {!function(string, boolean)} getProfileImageUrlCallback
+   */
+  ChangePicturePrivateApi.initialize = function(
+      getDefaultImagesCallback,
+      getSelectedImageUrlCallback,
+      getOldImageUrlCallback,
+      getProfileImageUrlCallback) {
+    ChangePicturePrivateApi.getDefaultImagesCallback_ =
+        getDefaultImagesCallback;
+    ChangePicturePrivateApi.getSelectedImageUrlCallback_ =
+        getSelectedImageUrlCallback;
+    ChangePicturePrivateApi.getOldImageUrlCallback_ = getOldImageUrlCallback;
+    ChangePicturePrivateApi.getProfileImageUrlCallback_ =
+        getProfileImageUrlCallback;
+    chrome.send('onChangePicturePageInitialized');
+  };
+
+  /**
+   * Called from C++ to provide the default set of images.
+   * @param {!Array<settings.DefaultImage>} images An array of default images.
+   */
+  ChangePicturePrivateApi.receiveDefaultImages = function(images) {
+    if (ChangePicturePrivateApi.getDefaultImagesCallback_)
+      ChangePicturePrivateApi.getDefaultImagesCallback_(images);
+  };
+
+  /**
+   * Called from C++ to provide the URL of the selected image.
+   * @param {string} imageUrl
+   */
+  ChangePicturePrivateApi.receiveSelectedImage = function(imageUrl) {
+    if (ChangePicturePrivateApi.getSelectedImageUrlCallback_)
+      ChangePicturePrivateApi.getSelectedImageUrlCallback_(imageUrl);
+  };
+
+  /**
+   * Called from C++ to provide the URL of the 'old' image. The 'old' image is
+   * any selected non-profile and non-default image. It can be from the
+   * camera, a file, or a deprecated default image. When this method is called,
+   * it's implied that the old image is selected.
+   * @param {string} imageUrl
+   */
+  ChangePicturePrivateApi.receiveOldImage = function(imageUrl) {
+    if (ChangePicturePrivateApi.getOldImageUrlCallback_)
+      ChangePicturePrivateApi.getOldImageUrlCallback_(imageUrl);
+  };
+
+  /**
+   * Called from C++ to provide the URL of the profile image.
+   * @param {string} imageUrl
+   * @param {boolean} selected
+   */
+  ChangePicturePrivateApi.receiveProfileImage = function(imageUrl, selected) {
+    if (ChangePicturePrivateApi.getProfileImageUrlCallback_)
+      ChangePicturePrivateApi.getProfileImageUrlCallback_(imageUrl, selected);
+  };
+
+  /**
+   * Called from C++ to notify page about camera presence.
+   * @param {boolean} cameraPresent
+   */
+  ChangePicturePrivateApi.receiveCameraPresence = function(cameraPresent) {
+    // TODO(tommycli): Implement camera functionality.
+  };
+
+  /**
+   * Called from JavaScript. Sets the user image to one of the default images.
+   * @param {string} imageUrl
+   */
+  ChangePicturePrivateApi.selectDefaultImage = function(imageUrl) {
+    chrome.send('selectImage', [imageUrl, 'default']);
+  };
+
+  /**
+   * Called from JavaScript. Sets the user image to the 'old' image.
+   */
+  ChangePicturePrivateApi.selectOldImage = function() {
+    chrome.send('selectImage', ['', 'old']);
+  };
+
+  /**
+   * Called from JavaScript. Sets the user image to the profile image.
+   */
+  ChangePicturePrivateApi.selectProfileImage = function() {
+    chrome.send('selectImage', ['', 'profile']);
+  };
+
+  return {
+    ChangePicturePrivateApi: ChangePicturePrivateApi,
+  };
+});