ui: Avoid animated user images in some parts of the UI.

This implements support for requesting a specific frame of an
image when using chrome://theme URLs. Initial support is limited
to frame 0, which is sufficient to conveniently provide stills
of animated images to Web UI.

Follow-up: Support for frame > 0 and scaling of animated images instead
of returning a scaled frame 0.

BUG=721647
TEST=Settings->People->"Change picture", only selected image is animating

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I9c920a6943cd1f1a2654716db73e8cd88575359d
Reviewed-on: https://chromium-review.googlesource.com/589709Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490623}

chrome/browser/resources/chromeos/login/oobe_change_picture.js

@@ -122,7 +122,7 @@ Polymer({
         this.sendSelectImage_(image.dataset.type, '');
         break;
       case CrPicture.SelectionTypes.DEFAULT:
-        this.sendSelectImage_(image.dataset.type, image.src);
+        this.sendSelectImage_(image.dataset.type, image.dataset.url);
         break;
       default:
         assertNotReached('Selected unknown image type');
@@ -196,7 +196,7 @@ Polymer({
    * @private
    */
   getImageSrc_: function(selectedItem) {
-    return (selectedItem && selectedItem.src) || '';
+    return (selectedItem && selectedItem.dataset.url) || '';
   },
 
   /**

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

@@ -177,7 +177,7 @@ Polymer({
           this.browserProxy_.selectOldImage();
         break;
       case CrPicture.SelectionTypes.DEFAULT:
-        this.browserProxy_.selectDefaultImage(image.src);
+        this.browserProxy_.selectDefaultImage(image.dataset.url);
         break;
       default:
         assertNotReached('Selected unknown image type');
@@ -237,7 +237,7 @@ Polymer({
    * @private
    */
   getImageSrc_: function(selectedItem) {
-    return (selectedItem && selectedItem.src) || '';
+    return (selectedItem && selectedItem.dataset.url) || '';
   },
 
   /**

chrome/browser/ui/webui/theme_source.cc

@@ -81,8 +81,10 @@ void ThemeSource::StartDataRequest(
     const content::URLDataSource::GotDataCallback& callback) {
   // Default scale factor if not specified.
   float scale = 1.0f;
+  // All frames by default if not specified.
+  int frame = -1;
   std::string parsed_path;
-  webui::ParsePathAndScale(GetThemeUrl(path), &parsed_path, &scale);
+  webui::ParsePathAndImageSpec(GetThemeUrl(path), &parsed_path, &scale, &frame);
 
   if (IsNewTabCssPath(parsed_path)) {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -132,14 +134,20 @@ void ThemeSource::StartDataRequest(
   const float max_scale = ui::GetScaleForScaleFactor(
       ResourceBundle::GetSharedInstance().GetMaxScaleFactor());
   const float unreasonable_scale = max_scale * 32;
-  if ((resource_id == -1) || (scale >= unreasonable_scale)) {
+  // TODO(reveman): Add support frames beyond 0 (crbug.com/750064).
+  if ((resource_id == -1) || (scale >= unreasonable_scale) || (frame > 0)) {
     // Either we have no data to send back, or the requested scale is
     // unreasonably large.  This shouldn't happen normally, as chrome://theme/
     // URLs are only used by WebUI pages and component extensions.  However, the
     // user can also enter these into the omnibox, so we need to fail
     // gracefully.
     callback.Run(nullptr);
-  } else if ((GetMimeType(path) == "image/png") && (scale > max_scale)) {
+  } else if ((GetMimeType(path) == "image/png") &&
+             ((scale > max_scale) || (frame != -1))) {
+    // This will extract and scale frame 0 of animated images.
+    // TODO(reveman): Support scaling of animated images and avoid scaling and
+    // re-encode when specific frame is specified (crbug.com/750064).
+    DCHECK_LE(frame, 0);
     SendThemeImage(callback, resource_id, scale);
   } else {
     SendThemeBitmap(callback, resource_id, scale);

ui/base/webui/web_ui_util.cc

@@ -88,15 +88,41 @@ bool ParseScaleFactor(const base::StringPiece& identifier,
   return true;
 }
 
-void ParsePathAndScale(const GURL& url,
-                       std::string* path,
-                       float* scale_factor) {
+// Parse a formatted frame index string into int and sets to |frame_index|.
+bool ParseFrameIndex(const base::StringPiece& identifier, int* frame_index) {
+  *frame_index = -1;
+  if (identifier.empty()) {
+    LOG(WARNING) << "Invalid frame index format: " << identifier;
+    return false;
+  }
+
+  if (*identifier.rbegin() != ']') {
+    LOG(WARNING) << "Invalid frame index format: " << identifier;
+    return false;
+  }
+
+  unsigned frame = 0;
+  if (!base::StringToUint(identifier.substr(0, identifier.length() - 1),
+                          &frame)) {
+    LOG(WARNING) << "Invalid frame index format: " << identifier;
+    return false;
+  }
+  *frame_index = static_cast<int>(frame);
+  return true;
+}
+
+void ParsePathAndImageSpec(const GURL& url,
+                           std::string* path,
+                           float* scale_factor,
+                           int* frame_index) {
   *path = net::UnescapeURLComponent(
       url.path().substr(1),
       net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
           net::UnescapeRule::SPACES);
   if (scale_factor)
     *scale_factor = 1.0f;
+  if (frame_index)
+    *frame_index = -1;
 
   // Detect and parse resource string ending in @<scale>x.
   std::size_t pos = path->rfind('@');
@@ -113,6 +139,33 @@ void ParsePathAndScale(const GURL& url,
     if (scale_factor)
       *scale_factor = factor;
   }
+
+  // Detect and parse resource string ending in [<frame>].
+  pos = path->rfind('[');
+  if (pos != std::string::npos) {
+    base::StringPiece stripped_path(*path);
+    int index;
+
+    if (ParseFrameIndex(
+            stripped_path.substr(pos + 1, stripped_path.length() - pos - 1),
+            &index)) {
+      // Strip frame index specification from path.
+      stripped_path.remove_suffix(stripped_path.length() - pos);
+      stripped_path.CopyToString(path);
+    }
+    if (frame_index)
+      *frame_index = index;
+  }
+}
+
+void ParsePathAndScale(const GURL& url,
+                       std::string* path,
+                       float* scale_factor) {
+  ParsePathAndImageSpec(url, path, scale_factor, nullptr);
+}
+
+void ParsePathAndFrame(const GURL& url, std::string* path, int* frame_index) {
+  ParsePathAndImageSpec(url, path, nullptr, frame_index);
 }
 
 void SetLoadTimeDataDefaults(const std::string& app_locale,

ui/base/webui/web_ui_util.h

@@ -34,16 +34,34 @@ UI_BASE_EXPORT std::string GetPngDataUrl(const unsigned char* data,
 UI_BASE_EXPORT WindowOpenDisposition
     GetDispositionFromClick(const base::ListValue* args, int start_index);
 
-// Pares a formatted scale factor string into float and sets to |scale_factor|.
+// Parse a formatted scale factor string into float and sets to |scale_factor|.
 UI_BASE_EXPORT bool ParseScaleFactor(const base::StringPiece& identifier,
                                      float* scale_factor);
 
+// Parses a URL containing some path [{frame}]@{scale}x. If it contains a
+// scale factor then it is returned and the associated part of the URL is
+// removed from the returned  |path|, otherwise the default scale factor is
+// returned and |path| is left intact. If it contains a frame index then it
+// is returned and the associated part of the URL is removed from the
+// returned |path|, otherwise the default frame index is returned and |path|
+// is left intact.
+UI_BASE_EXPORT void ParsePathAndImageSpec(const GURL& url,
+                                          std::string* path,
+                                          float* scale_factor,
+                                          int* frame_index);
+
 // Parses a URL containing some path @{scale}x. If it does not contain a scale
 // factor then the default scale factor is returned.
 UI_BASE_EXPORT void ParsePathAndScale(const GURL& url,
                                       std::string* path,
                                       float* scale_factor);
 
+// Parses a URL containing some path [{frame}]. If it does not contain a frame
+// index then the default frame index is returned.
+UI_BASE_EXPORT void ParsePathAndFrame(const GURL& url,
+                                      std::string* path,
+                                      int* frame_index);
+
 // Helper function to set some default values (e.g., font family, size,
 // language, and text direction) into the given dictionary. Requires an
 // application locale (i.e. g_browser_process->GetApplicationLocale()).

ui/base/webui/web_ui_util_unittest.cc

@@ -52,3 +52,103 @@ TEST(WebUIUtilTest, ParsePathAndScale) {
   EXPECT_EQ("random/username/and/more", path);
   EXPECT_EQ(1.3f, factor);
 }
+
+TEST(WebUIUtilTest, ParsePathAndFrame) {
+  std::string path;
+
+  int index = -2;
+  GURL url("http://[::192.9.5.5]/and/some/more");
+  webui::ParsePathAndFrame(url, &path, &index);
+  EXPECT_EQ("and/some/more", path);
+  EXPECT_EQ(-1, index);
+
+  index = -2;
+  GURL url2("http://host/and/some/more");
+  webui::ParsePathAndFrame(url2, &path, &index);
+  EXPECT_EQ("and/some/more", path);
+  EXPECT_EQ(-1, index);
+
+  index = -2;
+  GURL url3("http://host/and/some/more[2a]");
+  webui::ParsePathAndFrame(url3, &path, &index);
+  EXPECT_EQ("and/some/more[2a]", path);
+  EXPECT_EQ(-1, index);
+
+  index = -2;
+  GURL url4("http://host/and/some/more[]");
+  webui::ParsePathAndFrame(url4, &path, &index);
+  EXPECT_EQ("and/some/more[]", path);
+  EXPECT_EQ(-1, index);
+
+  index = -2;
+  GURL url5("http://host/and/some/more[-2]");
+  webui::ParsePathAndFrame(url5, &path, &index);
+  EXPECT_EQ("and/some/more[-2]", path);
+  EXPECT_EQ(-1, index);
+
+  index = -2;
+  GURL url6("http://[::192.9.5.5]/and/some/more[0]");
+  webui::ParsePathAndFrame(url6, &path, &index);
+  EXPECT_EQ("and/some/more", path);
+  EXPECT_EQ(0, index);
+
+  index = -2;
+  GURL url7("http://host/and/some/more[1]");
+  webui::ParsePathAndFrame(url7, &path, &index);
+  EXPECT_EQ("and/some/more", path);
+  EXPECT_EQ(1, index);
+
+  index = -2;
+  GURL url8("http://host/and/some/more[3]");
+  webui::ParsePathAndFrame(url8, &path, &index);
+  EXPECT_EQ("and/some/more", path);
+  EXPECT_EQ(3, index);
+
+  index = -2;
+  GURL url9("http://host/and/some/more0]");
+  webui::ParsePathAndFrame(url9, &path, &index);
+  EXPECT_EQ("and/some/more0]", path);
+  EXPECT_EQ(-1, index);
+
+  index = -2;
+  GURL url10("http://host/and/some/more[0");
+  webui::ParsePathAndFrame(url10, &path, &index);
+  EXPECT_EQ("and/some/more[0", path);
+  EXPECT_EQ(-1, index);
+}
+
+TEST(WebUIUtilTest, ParsePathAndImageSpec) {
+  std::string path;
+
+  float factor = 0;
+  int index = -2;
+  GURL url("http://[::192.9.5.5]/some/random/username@email/and/more");
+  webui::ParsePathAndImageSpec(url, &path, &factor, &index);
+  EXPECT_EQ("some/random/username@email/and/more", path);
+  EXPECT_EQ(1.0f, factor);
+  EXPECT_EQ(-1, index);
+
+  factor = 0;
+  index = -2;
+  GURL url2("http://host/some/random/username/and/more");
+  webui::ParsePathAndImageSpec(url2, &path, &factor, &index);
+  EXPECT_EQ("some/random/username/and/more", path);
+  EXPECT_EQ(1.0f, factor);
+  EXPECT_EQ(-1, index);
+
+  factor = 0;
+  index = -2;
+  GURL url3("http://host/some/random/username/and/more[0]@2x");
+  webui::ParsePathAndImageSpec(url3, &path, &factor, &index);
+  EXPECT_EQ("some/random/username/and/more", path);
+  EXPECT_EQ(2.0f, factor);
+  EXPECT_EQ(0, index);
+
+  factor = 0;
+  index = -2;
+  GURL url4("http://[::192.9.5.5]/some/random/username@email/and/more[1]@1.5x");
+  webui::ParsePathAndImageSpec(url4, &path, &factor, &index);
+  EXPECT_EQ("some/random/username@email/and/more", path);
+  EXPECT_EQ(1.5f, factor);
+  EXPECT_EQ(1, index);
+}

ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.html

@@ -69,13 +69,16 @@
         <!-- Shows and selects the current profile picture. -->
         <img id="profileImage" role="radio"
             data-type$="[[selectionTypesEnum_.PROFILE]]"
-            src="[[profileImageUrl_]]" hidden="[[!profileImageUrl_]]"
+            data-url$="[[profileImageUrl_]]"
+            src="[[getImgSrc_(profileImageUrl_)]]"
+            hidden="[[!profileImageUrl_]]"
             srcset="[[getImgSrc2x_(profileImageUrl_)]]"
             title="[[profileImageLoadingLabel]]">
         <!-- Shows and selects the previously selected ('old') picture. -->
         <img id="oldImage" role="radio"
             data-type$="[[selectionTypesEnum_.OLD]]"
             data-image-index$="[[oldImageIndex_]]"
+            data-url$="[[oldImageUrl_]]"
             src="[[oldImageUrl_]]" hidden="[[!oldImageUrl_]]"
             title="[[oldImageLabel]]">
         <!-- Shows the list of available images to select from. -->
@@ -83,7 +86,8 @@
           <img role="radio"
               data-type$="[[selectionTypesEnum_.DEFAULT]]"
               data-index$="[[index]]" data-image-index$="[[item.index]]"
-              src="[[item.url]]"
+              data-url$="[[item.url]]"
+              src="[[getImgSrc_(item.url)]]"
               srcset="[[getImgSrc2x_(item.url)]]" title="[[item.title]]">
         </template>
       </iron-selector>

ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_list.js

@@ -113,7 +113,7 @@ Polymer({
    */
   setSelectedImageUrl(imageUrl) {
     var image = this.$.selector.items.find(function(image) {
-      return image.src == imageUrl;
+      return image.dataset.url == imageUrl;
     });
     if (image) {
       this.setSelectedImage_(image);
@@ -220,6 +220,20 @@ Polymer({
     this.selectImage_(event.detail.item, activate);
   },
 
+  /**
+   * Returns the image to use for 'src'.
+   * @param {string} url
+   * @return {string}
+   * @private
+   */
+  getImgSrc_: function(url) {
+    // Use first frame of animated user images.
+    if (url.startsWith('chrome://theme'))
+      return url + '[0]';
+
+    return url;
+  },
+
   /**
    * Returns the 2x (high dpi) image to use for 'srcset' for chrome://theme
    * images. Note: 'src' will still be used as the 1x candidate as per the HTML
@@ -229,8 +243,8 @@ Polymer({
    * @private
    */
   getImgSrc2x_: function(url) {
-    if (url.indexOf('chrome://theme') != 0)
+    if (!url.startsWith('chrome://theme'))
       return '';
-    return url + '@2x 2x';
+    return url + '[0]@2x 2x';
   },
 });

ui/webui/resources/cr_elements/chromeos/cr_picture/cr_picture_pane.js

@@ -100,7 +100,7 @@ Polymer({
    */
   getImgSrc_: function(url) {
     // Always use 2x user image for preview.
-    if (url.indexOf('chrome://theme') == 0)
+    if (url.startsWith('chrome://theme'))
       return url + '@2x';
 
     return url;