[webui][ntp] Fix doodle share button position, size for scaled doodles

Previously, we set the share button position to the position sent in the
doodle response and hardcoded the share button size to 26x26px. This
works fine as long as the doodle is shown with the same dimensions as
its intrinsic size. However, we limit the doodle height to 230px (or
potentially more in the future for boxed themed mode doodles) and scale
the doodle image accordingly. Therefore, the share button can end up
at a different position and size relative to the doodle image as
originally intended. This CL fixes that by setting the share button size
and position as a percentage of the intrinsic doodle image size.

Bug: 688960
Change-Id: I55261f46d9ae074b81c03a7563bc068b1fd247bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2250512
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Esmael Elmoslimany <aee@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780029}

chrome/browser/resources/new_tab_page/logo.html

@@ -24,9 +24,16 @@
     background-image: url(chrome://resources/images/google_logo.svg);
   }
 
+  #doodle {
+    display: flex;
+    flex-direction: column;
+    height: 100%;
+    justify-content: center;
+  }
+
   #imageContainer {
     cursor: pointer;
-    display: grid;
+    display: flex;
     height: fit-content;
     outline: none;
     position: relative;
@@ -37,11 +44,6 @@
     box-shadow: 0 0 0 2px rgba(var(--google-blue-600-rgb), .4);
   }
 
-  #imageContainer > * {
-    grid-column-start: 1;
-    grid-row-start: 1;
-  }
-
   #image {
     max-height: var(--ntp-logo-height);
     max-width: 100%;
@@ -50,24 +52,33 @@
   #animation {
     height: 100%;
     pointer-events: none;
+    position: absolute;
     width: 100%;
   }
 
   #shareButton {
+    background-color: var(--ntp-logo-share-button-background-color, none);
     border: none;
-    height: 26px;
-    min-width: 26px;
+    height: var(--ntp-logo-share-button-height, 0);
+    left: var(--ntp-logo-share-button-x, 0);
+    min-width: var(--ntp-logo-share-button-width, 0);
     opacity: 0.8;
     outline: initial;
     padding: 2px;
     position: absolute;
-    width: 26px;
+    top: var(--ntp-logo-share-button-y, 0);
+    width: var(--ntp-logo-share-button-width, 0);
   }
 
   #shareButton:hover {
     opacity: 1;
   }
 
+  #shareButton img {
+    height: 100%;
+    width: 100%;
+  }
+
   #iframe {
     border: none;
     height: var(--height, --ntp-logo-height);
@@ -98,10 +109,7 @@
             hidden="[[!showAnimation_]]">
         </ntp-untrusted-iframe>
         <cr-button id="shareButton" title="$i18n{shareDoodle}"
-            on-click="onShareButtonClick_"
-            style="background-color: [[rgbaOrUnset_(doodle_.content.imageDoodle.shareButton.backgroundColor)]];
-                   left: [[doodle_.content.imageDoodle.shareButton.x]]px;
-                   top: [[doodle_.content.imageDoodle.shareButton.y]]px;">
+            on-click="onShareButtonClick_">
           <img id="shareButtonImage"
               src="[[doodle_.content.imageDoodle.shareButton.iconUrl.url]]">
           </img>

chrome/browser/resources/new_tab_page/logo.js

@@ -13,6 +13,9 @@ import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/poly
 import {BrowserProxy} from './browser_proxy.js';
 import {skColorToRgba} from './utils.js';
 
+/** @type {number} */
+const SHARE_BUTTON_SIZE_PX = 26;
+
 // Shows the Google logo or a doodle if available.
 class LogoElement extends PolymerElement {
   static get is() {
@@ -49,7 +52,10 @@ class LogoElement extends PolymerElement {
       loaded_: Boolean,
 
       /** @private {newTabPage.mojom.Doodle} */
-      doodle_: Object,
+      doodle_: {
+        observer: 'onDoodleChange_',
+        type: Object,
+      },
 
       /** @private */
       canShowDoodle_: {
@@ -164,6 +170,23 @@ class LogoElement extends PolymerElement {
     performance.measure('logo-creation', 'logo-creation-start');
   }
 
+  /** @private */
+  onDoodleChange_() {
+    const imageDoodle = this.doodle_ && this.doodle_.content.imageDoodle;
+    this.updateStyles({
+      '--ntp-logo-share-button-background-color':
+          imageDoodle && skColorToRgba(imageDoodle.shareButton.backgroundColor),
+      '--ntp-logo-share-button-height':
+          imageDoodle && `${SHARE_BUTTON_SIZE_PX / imageDoodle.height * 100}%`,
+      '--ntp-logo-share-button-width':
+          imageDoodle && `${SHARE_BUTTON_SIZE_PX / imageDoodle.width * 100}%`,
+      '--ntp-logo-share-button-x': imageDoodle &&
+          `${imageDoodle.shareButton.x / imageDoodle.width * 100}%`,
+      '--ntp-logo-share-button-y': imageDoodle &&
+          `${imageDoodle.shareButton.y / imageDoodle.height * 100}%`,
+    });
+  }
+
   /**
    * @return {boolean}
    * @private
@@ -314,15 +337,6 @@ class LogoElement extends PolymerElement {
         '';
   }
 
-  /**
-   * @param {skia.mojom.SkColor} skColor
-   * @return {string}
-   * @private
-   */
-  rgbaOrUnset_(skColor) {
-    return skColor ? skColorToRgba(skColor) : 'unset';
-  }
-
   /**
    * @param {!Event} e
    * @private

chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom

@@ -202,6 +202,9 @@ struct ImageDoodleContent {
   url.mojom.Url on_click_url;
   // URL pointing to animated content (e.g. gif). Only set for animated doodles.
   url.mojom.Url? animation_url;
+  // Dimensions of the original image in pixels.
+  uint32 width;
+  uint32 height;
   // Specification of the share button.
   DoodleShareButton share_button;
   // URL displayed to users, which they can use to share the doodle.

chrome/browser/ui/webui/new_tab_page/new_tab_page_handler.cc

@@ -1229,6 +1229,8 @@ void NewTabPageHandler::OnLogoAvailable(
     if (logo->metadata.type == search_provider_logos::LogoType::ANIMATED) {
       image_doodle_content->animation_url = logo->metadata.animated_url;
     }
+    image_doodle_content->width = logo->metadata.width_px;
+    image_doodle_content->height = logo->metadata.height_px;
     image_doodle_content->share_button =
         new_tab_page::mojom::DoodleShareButton::New();
     image_doodle_content->share_button->x = logo->metadata.share_button_x;

chrome/test/data/webui/new_tab_page/logo_test.js

@@ -6,11 +6,46 @@ import {$$, BrowserProxy} from 'chrome://new-tab-page/new_tab_page.js';
 import {assertNotStyle, assertStyle, createTestProxy, keydown} from 'chrome://test/new_tab_page/test_support.js';
 import {eventToPromise, flushTasks} from 'chrome://test/test_util.m.js';
 
-function createImageDoodle() {
+/**
+ * @param {!Element} element
+ * @param {!Element} reference
+ * @return {!{top: number, right: number, bottom: number, left: number}}
+ */
+function getRelativePosition(element, reference) {
+  const referenceRect = reference.getBoundingClientRect();
+  const elementRect = element.getBoundingClientRect();
+  return {
+    top: elementRect.top - referenceRect.top,
+    right: elementRect.right - referenceRect.right,
+    bottom: elementRect.bottom - referenceRect.bottom,
+    left: elementRect.left - referenceRect.left,
+  };
+}
+
+/**
+ * @param {number} width
+ * @param {number} height
+ * @return {string}
+ */
+function createImageDataUrl(width, height) {
+  const canvas = document.createElement('canvas');
+  canvas.width = width;
+  canvas.height = height;
+  return canvas.toDataURL('image/png');
+}
+
+/**
+ * @param {number} width
+ * @param {number} height
+ * @return {!newTabPage.mojom.Doodle}
+ */
+function createImageDoodle(width, height) {
+  width = width || 500;
+  height = height || 200;
   return {
     content: {
       imageDoodle: {
-        imageUrl: {url: 'data:foo'},
+        imageUrl: {url: createImageDataUrl(width, height)},
         onClickUrl: {url: 'https://foo.com'},
         shareButton: {
           backgroundColor: {value: 0xFFFF0000},
@@ -19,6 +54,8 @@ function createImageDoodle() {
           iconUrl: {url: 'data:bar'},
         },
         imageImpressionLogUrl: {url: 'https://log.com'},
+        width,
+        height,
       }
     }
   };
@@ -55,12 +92,11 @@ suite('NewTabPageLogoTest', () => {
 
   test('setting simple doodle shows image', async () => {
     // Arrange.
-    const doodle = createImageDoodle();
-    doodle.content.imageDoodle.imageUrl = {url: 'data:foo'};
+    const doodle = createImageDoodle(/*width=*/ 500, /*height=*/ 200);
     doodle.content.imageDoodle.shareButton = {
       backgroundColor: {value: 0xFFFF0000},
-      x: 11,
-      y: 12,
+      x: 10,
+      y: 20,
       iconUrl: {url: 'data:bar'},
     };
 
@@ -70,17 +106,44 @@ suite('NewTabPageLogoTest', () => {
     // Assert.
     assertNotStyle($$(logo, '#doodle'), 'display', 'none');
     assertEquals($$(logo, '#logo'), null);
-    assertEquals($$(logo, '#image').src, 'data:foo');
+    assertEquals(
+        $$(logo, '#image').src, doodle.content.imageDoodle.imageUrl.url);
     assertNotStyle($$(logo, '#image'), 'display', 'none');
+    assertEquals(500, $$(logo, '#image').offsetWidth);
+    assertEquals(200, $$(logo, '#image').offsetHeight);
     assertNotStyle($$(logo, '#shareButton'), 'display', 'none');
     assertStyle($$(logo, '#shareButton'), 'background-color', 'rgb(255, 0, 0)');
-    assertStyle($$(logo, '#shareButton'), 'left', '11px');
-    assertStyle($$(logo, '#shareButton'), 'top', '12px');
+    const shareButtonPosition =
+        getRelativePosition($$(logo, '#shareButton'), $$(logo, '#image'));
+    assertEquals(10, shareButtonPosition.left);
+    assertEquals(20, shareButtonPosition.top);
+    assertEquals(26, $$(logo, '#shareButton').offsetWidth);
+    assertEquals(26, $$(logo, '#shareButton').offsetHeight);
     assertEquals($$(logo, '#shareButtonImage').src, 'data:bar');
     assertStyle($$(logo, '#animation'), 'display', 'none');
     assertFalse(!!$$(logo, '#iframe'));
   });
 
+  test('setting too large image doodle resizes image', async () => {
+    // Arrange.
+    const doodle = createImageDoodle(/*width=*/ 1000, /*height=*/ 500);
+    doodle.content.imageDoodle.shareButton.x = 10;
+    doodle.content.imageDoodle.shareButton.y = 20;
+
+    // Act.
+    const logo = await createLogo(doodle);
+
+    // Assert.
+    assertEquals(460, $$(logo, '#image').offsetWidth);
+    assertEquals(230, $$(logo, '#image').offsetHeight);
+    const shareButtonPosition =
+        getRelativePosition($$(logo, '#shareButton'), $$(logo, '#image'));
+    assertEquals(5, Math.round(shareButtonPosition.left));
+    assertEquals(9, Math.round(shareButtonPosition.top));
+    assertEquals(12, $$(logo, '#shareButton').offsetWidth);
+    assertEquals(12, $$(logo, '#shareButton').offsetHeight);
+  });
+
   test('setting animated doodle shows image', async () => {
     // Arrange.
     const doodle = createImageDoodle();
@@ -276,6 +339,9 @@ suite('NewTabPageLogoTest', () => {
     assertNotStyle($$(logo, '#image'), 'display', 'none');
     assertNotStyle($$(logo, '#animation'), 'display', 'none');
     assertEquals($$(logo, '#animation').path, 'image?https://foo.com');
+    assertDeepEquals(
+        $$(logo, '#image').getBoundingClientRect(),
+        $$(logo, '#animation').getBoundingClientRect());
   });
 
   test('clicking animation of animated doodle opens link', async () => {

components/search_provider_logos/google_logo_api.cc

@@ -248,11 +248,17 @@ std::unique_ptr<EncodedLogo> ParseDoodleLogoResponse(
 
   if (is_simple || is_animated) {
     const base::DictionaryValue* image = nullptr;
-    std::string bg_color;
-    if (ddljson->GetDictionary("dark_large_image", &image)) {
-      image->GetString("background_color", &bg_color);
+    if (ddljson->GetDictionary("large_image", &image)) {
+      image->GetInteger("width", &logo->metadata.width_px);
+      image->GetInteger("height", &logo->metadata.height_px);
+    }
+    const base::DictionaryValue* dark_image = nullptr;
+    if (ddljson->GetDictionary("dark_large_image", &dark_image)) {
+      dark_image->GetString("background_color",
+                            &logo->metadata.dark_background_color);
+      dark_image->GetInteger("width", &logo->metadata.dark_width_px);
+      dark_image->GetInteger("height", &logo->metadata.dark_height_px);
     }
-    logo->metadata.dark_background_color = bg_color;
   }
 
   const bool is_eligible_for_share_button =

components/search_provider_logos/google_logo_api_unittest.cc

@@ -394,7 +394,8 @@ TEST(GoogleNewLogoApiTest, ParsesAnimatedImage) {
     },
     "dark_large_image": {
       "is_animated_gif": true,
-      "url": "https://www.doodle.com/dark_image.gif"
+      "url": "https://www.doodle.com/dark_image.gif",
+      "background_color": "#ABCDEF"
     },
     "cta_data_uri": "data:image/png;base64,YWJj",
     "dark_cta_data_uri": "data:image/png;base64,eHl6"
@@ -411,6 +412,7 @@ TEST(GoogleNewLogoApiTest, ParsesAnimatedImage) {
             logo->metadata.animated_url);
   EXPECT_EQ(GURL("https://www.doodle.com/dark_image.gif"),
             logo->metadata.dark_animated_url);
+  EXPECT_EQ("#ABCDEF", logo->metadata.dark_background_color);
   EXPECT_EQ("abc", logo->encoded_image->data());
   EXPECT_EQ("xyz", logo->dark_encoded_image->data());
   EXPECT_EQ(LogoType::ANIMATED, logo->metadata.type);
@@ -442,6 +444,33 @@ TEST(GoogleNewLogoApiTest, ParsesLoggingUrls) {
   EXPECT_EQ(GURL("https://base.doo/ctalog?c=d"), logo->metadata.cta_log_url);
 }
 
+TEST(GoogleNewLogoApiTest, ParsesImageSize) {
+  const GURL base_url("https://base.doo/");
+  const std::string json = R"json()]}'
+{
+  "ddljson": {
+    "large_image": {
+      "width": 500,
+      "height": 200
+    },
+    "dark_large_image": {
+      "width": 600,
+      "height": 230
+    }
+  }
+})json";
+
+  bool failed = false;
+  std::unique_ptr<EncodedLogo> logo = ParseDoodleLogoResponse(
+      base_url, std::make_unique<std::string>(json), base::Time(), &failed);
+
+  ASSERT_FALSE(failed);
+  EXPECT_EQ(500, logo->metadata.width_px);
+  EXPECT_EQ(200, logo->metadata.height_px);
+  EXPECT_EQ(600, logo->metadata.dark_width_px);
+  EXPECT_EQ(230, logo->metadata.dark_height_px);
+}
+
 TEST(GoogleNewLogoApiTest, ParsesInteractiveDoodle) {
   const GURL base_url("https://base.doo/");
   const std::string json = R"json()]}'

components/search_provider_logos/logo_cache.cc

@@ -39,6 +39,10 @@ const char kDarkAnimatedUrlKey[] = "dark_animated_url";
 const char kLogUrlKey[] = "log_url";
 const char kCtaLogUrlKey[] = "cta_log_url";
 const char kShortLinkKey[] = "short_link";
+const char kWidthPx[] = "width_px";
+const char kHeightPx[] = "height_px";
+const char kDarkWidthPx[] = "dark_width_px";
+const char kDarkHeightPx[] = "dark_height_px";
 const char kIframeWidthPx[] = "iframe_width_px";
 const char kIframeHeightPx[] = "iframe_height_px";
 const char kDarkBackgroundColorKey[] = "dark_background_color";
@@ -252,6 +256,10 @@ std::unique_ptr<LogoMetadata> LogoCache::LogoMetadataFromString(
       !dict->GetString(kDarkShareButtonIcon,
                        &metadata->dark_share_button_icon) ||
       !dict->GetString(kDarkShareButtonBg, &metadata->dark_share_button_bg) ||
+      !dict->GetInteger(kWidthPx, &metadata->width_px) ||
+      !dict->GetInteger(kHeightPx, &metadata->height_px) ||
+      !dict->GetInteger(kDarkWidthPx, &metadata->dark_width_px) ||
+      !dict->GetInteger(kDarkHeightPx, &metadata->dark_height_px) ||
       !dict->GetInteger(kIframeWidthPx, &metadata->iframe_width_px) ||
       !dict->GetInteger(kIframeHeightPx, &metadata->iframe_height_px) ||
       !dict->GetString(kDarkBackgroundColorKey,
@@ -305,6 +313,10 @@ void LogoCache::LogoMetadataToString(const LogoMetadata& metadata,
   dict.SetDouble(kDarkShareButtonOpacity, metadata.dark_share_button_opacity);
   dict.SetString(kDarkShareButtonIcon, metadata.dark_share_button_icon);
   dict.SetString(kDarkShareButtonBg, metadata.dark_share_button_bg);
+  dict.SetInteger(kWidthPx, metadata.width_px);
+  dict.SetInteger(kHeightPx, metadata.height_px);
+  dict.SetInteger(kDarkWidthPx, metadata.dark_width_px);
+  dict.SetInteger(kDarkHeightPx, metadata.dark_height_px);
   dict.SetInteger(kIframeWidthPx, metadata.iframe_width_px);
   dict.SetInteger(kIframeHeightPx, metadata.iframe_height_px);
   dict.SetString(kDarkBackgroundColorKey, metadata.dark_background_color);

components/search_provider_logos/logo_cache_unittest.cc

@@ -49,6 +49,10 @@ LogoMetadata GetExampleMetadata() {
   metadata.dark_share_button_opacity = 0.7;
   metadata.dark_share_button_icon = "dark_test_img";
   metadata.dark_share_button_bg = "#22ff22";
+  metadata.width_px = 500;
+  metadata.height_px = 200;
+  metadata.dark_width_px = 600;
+  metadata.dark_height_px = 230;
   return metadata;
 }
 
@@ -134,6 +138,15 @@ void ExpectMetadataEqual(const LogoMetadata& expected_metadata,
             actual_metadata.dark_share_button_icon);
   EXPECT_EQ(expected_metadata.dark_share_button_bg,
             actual_metadata.dark_share_button_bg);
+  EXPECT_EQ(expected_metadata.width_px, actual_metadata.width_px);
+  EXPECT_EQ(expected_metadata.height_px, actual_metadata.height_px);
+  EXPECT_EQ(expected_metadata.dark_width_px, actual_metadata.dark_width_px);
+  EXPECT_EQ(expected_metadata.dark_height_px, actual_metadata.dark_height_px);
+  EXPECT_EQ(expected_metadata.iframe_width_px, actual_metadata.iframe_width_px);
+  EXPECT_EQ(expected_metadata.iframe_height_px,
+            actual_metadata.iframe_height_px);
+  EXPECT_EQ(expected_metadata.dark_background_color,
+            actual_metadata.dark_background_color);
 }
 
 void ExpectLogosEqual(const EncodedLogo& expected_logo,

components/search_provider_logos/logo_common.h

@@ -86,6 +86,13 @@ struct LogoMetadata {
   // The URL used for sharing doodles.
   GURL short_link;
 
+  // SIMPLE, ANIMATED: original dimensions of the image.
+  // INTERACTIVE: not used.
+  int width_px = 0;
+  int height_px = 0;
+  int dark_width_px = 0;
+  int dark_height_px = 0;
+
   // SIMPLE, ANIMATED: ignored
   // INTERACTIVE: appropriate dimensions for the iframe.
   int iframe_width_px = 0;