Fix coordinate calculations under pinch-zoom

telemetry/telemetry/internal/actions/gesture_common.js

 // Copyright 2013 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.
 
 // This file provides common functionality for synthetic gesture actions.
 'use strict';
 
 (function() {
 
   // Make sure functions are injected only once.
   if (window.__GestureCommon_GetBoundingVisibleRect)
     return;
 
-  // Returns the bounding rectangle wrt to the top-most document.
+  // Returns the bounding rectangle wrt to the layout viewport.
   function getBoundingRect(el) {
     var clientRect = el.getBoundingClientRect();
     var bound = { left: clientRect.left,
                   top: clientRect.top,
                   width: clientRect.width,
                   height: clientRect.height };
 
     var frame = el.ownerDocument.defaultView.frameElement;
     while (frame) {
       var frameBound = frame.getBoundingClientRect();
       // This computation doesn't account for more complex CSS transforms on the
       // frame (e.g. scaling or rotations).
       bound.left += frameBound.left;
       bound.top += frameBound.top;
 
       frame = frame.ownerDocument.frameElement;
     }
     return bound;
   }
 
-  // TODO(ulan): Remove this function once
-  // chrome.gpuBenchmarking.pageScaleFactor is available in reference builds.
-  function getPageScaleFactor() {
-    if (chrome.gpuBenchmarking.pageScaleFactor)
-      return chrome.gpuBenchmarking.pageScaleFactor();
-    return 1;
-  }
-
   // Zoom-independent window height. See crbug.com/627123 for more details.
   function getWindowHeight() {
-    return getPageScaleFactor() * chrome.gpuBenchmarking.visualViewportHeight();
+    return chrome.gpuBenchmarking.pageScaleFactor() *
+        chrome.gpuBenchmarking.visualViewportHeight();
   }
 
   // Zoom-independent window width. See crbug.com/627123 for more details.
   function getWindowWidth() {
-    return getPageScaleFactor() * chrome.gpuBenchmarking.visualViewportWidth();
+    return chrome.gpuBenchmarking.pageScaleFactor() *
+        chrome.gpuBenchmarking.visualViewportWidth();
   }
 
   function clamp(min, value, max) {
     return Math.min(Math.max(min, value), max);
   }
 
+  // Returns the bounding rect in the visual viewport's coordinates.
   function getBoundingVisibleRect(el) {
     // Get the element bounding rect.
     var rect = getBoundingRect(el);
 
+    // TODO(bokan): Remove this once gpuBenchmarking is changed to take all
+    // coordinates in viewport space. crbug.com/610021.
+    if ('gesturesExpectedInViewportCoordinates' in chrome.gpuBenchmarking) {

[ericrk, 2017/09/15 16:41:04]

So, to make sure I understand, this code is the new code that will run once
https://chromium-review.googlesource.com/c/chromium/src/+/668376 lands? So is
this the permanent code, and once everything's done the "if ('gesture...." will
be gone but the code block will remain?

[bokan, 2017/09/17 05:48:51]

That's right. Once that CL lands the code guarded by
gesturesExpectedInViewportCoordinates will be in effect so I'll remove the guard
here (and the `else` block in pinch.js). I'll then remove
gesturesExpectedInViewportCoordinates from gpuBenchmarking.

+      // Apply the visual viewport transform (i.e. pinch-zoom) to the bounding
+      // rect.
+      const scale = chrome.gpuBenchmarking.pageScaleFactor();
+      const visualViewportX = chrome.gpuBenchmarking.visualViewportX();
+      const visualViewportY = chrome.gpuBenchmarking.visualViewportY();
+      rect.top = (rect.top - visualViewportY) * scale;
+      rect.left = (rect.left - visualViewportX) * scale;
+      rect.width *= scale;

[ericrk, 2017/09/15 16:41:04]

So, I think your code is correct, but I want to make sure I'm understanding
correctly:

chrome.gpuBenchmarking.visualViewportX/Y return a value in non-scaled page
coordinates, meaning that when we subtract / scale below, everything works out.
This isn't immediately intuitive to me, as it feels like these values could
either be pre or post-scale. Can you maybe add a comment indicating that
visualViewportX/Y are non-scaled coordinates in the page?

[bokan, 2017/09/17 05:48:51]

Your understanding is correct. These values are in CSS pixels, when we scale the
page, we're changing the size of the CSS pixel so the values themselves don't
change. Width/Height are also in CSS pixels but they change as we scale because
the physical viewport is a fixed size so fewer or more CSS pixels now fit into
it.

Thanks for pointing out the lack of documentation, I sometimes forget this stuff
isn't intimitely familiar to anyone but me :). I've added documentation here and
in gpuBenchmarking.

+      rect.height *= scale;
+    }
+
     // Get the window dimensions.
-    var windowHeight = getWindowHeight();
-    var windowWidth = getWindowWidth();
+    const windowHeight = getWindowHeight();
+    const windowWidth = getWindowWidth();
 
     // Then clip the rect to the screen size.
     rect.top = clamp(0, rect.top, windowHeight);
     rect.left = clamp(0, rect.left, windowWidth);
     rect.height = clamp(0, rect.height, windowHeight - rect.top);
     rect.width = clamp(0, rect.width, windowWidth - rect.left);
 
     return rect;
   }
 
   window.__GestureCommon_GetBoundingVisibleRect = getBoundingVisibleRect;
   window.__GestureCommon_GetWindowHeight = getWindowHeight;
   window.__GestureCommon_GetWindowWidth = getWindowWidth;
 })();