New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 599656 link

Starred by 8 users

Performance test smoothness.top_25_smooth failing on samus, link and chell devices

Project Member Reported by haddowk@chromium.org, Mar 31 2016

Issue description


The test is crashing so no metrics are recorded for :

smoothness.top_25_smooth/percentage_smooth
smoothness.top_25_smooth/mean_frame_time
smoothness.top_25_smooth/mean_input_event_latency

Last good run on Samus 51.8097.0.0
Last good run on Link 51.8099.0.0 
Last good run on Chell 51.8082.0.0

Last good test run :

https://pantheon.corp.google.com/storage/browser/chromeos-autotest-results/58535225-haddowk/chromeos2-row24-rack1-host9/telemetry_Benchmarks.smoothness.top_25_smooth/debug/

 File "/home/chromeos-test/images/samus-release/R51-8136.0.0/telemetry_src/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 596, in recv
    opcode, data = self.recv_data()
  File "/home/chromeos-test/images/samus-release/R51-8136.0.0/telemetry_src/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 606, in recv_data
    frame = self.recv_frame()
  File "/home/chromeos-test/images/samus-release/R51-8136.0.0/telemetry_src/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 637, in recv_frame
    self._frame_header = self._recv_strict(2)
  File "/home/chromeos-test/images/samus-release/R51-8136.0.0/telemetry_src/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 746, in _recv_strict
    bytes = self._recv(shortage)
  File "/home/chromeos-test/images/samus-release/R51-8136.0.0/telemetry_src/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 739, in _recv
    raise WebSocketConnectionClosedException()
DevtoolsTargetCrashException: Devtools target crashed
********************************************************************************
(/home/chromeos-test/images/samus-release/R51-8136.0.0/telemetry_src/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:320 _ConvertExceptionFromInspectorWebsocket) Original exception:

********************************************************************************
(/home/chromeos-test/images/samus-release/R51-8136.0.0/telemetry_src/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:341 _AddDebuggingInformation) Received a socket error in the browser connection and the tab no longer exists. The tab probably crashed.
********************************************************************************
(/home/chromeos-test/images/samus-release/R51-8136.0.0/telemetry_src/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py:342 _AddDebuggingInformation) Debugger url: ws://127.0.0.1:41899/devtools/page/130370AD-38DA-494A-8D4A-676433F5668F
Stack Trace:
********************************************************************************
	Cannot get stack trace on CrOS
********************************************************************************
Standard output:
********************************************************************************
	Cannot get standard output on CrOS
********************************************************************************

 
Cc: mshe...@chromium.org
Owner: bccheng@chromium.org
Ben - often telemetry crashes end in similar - if anything can be done to get better tracing of the real problem from the device it would help get bugs targeted to the people who can actually make the fix.
Status: Assigned (was: Untriaged)
Status: Started (was: Assigned)
The problem first shows up in R51.8100, and only on Samus but not on Falco.
The regression was introduced by upreving Chrome to 51.0.2687.0. Bisecting is underway but there is something weird with Ninja build so building Chrome takes 5 hours. Currently there are 40 commits left to find the offending one.
Issue 605175 has been merged into this issue.
Cc: bccheng@chromium.org
Owner: osh...@chromium.org
Through bisecting CLs I found the failure was introduced by this CL:

commit 6108f2e839b14fea7583b501d7e3e1addfc6af8a
Author: oshima <oshima@chromium.org>
Date:   Mon Mar 21 14:19:32 2016 -0700

    Revert of Disable use-zoom-for-dsf flag for m50 (patchset #1 id:1 of https://codereview.chromium.org/1788653005/ )

    Reason for revert:
    Reenable this on m51.

    Original issue's description:
    > Disable use-zoom-for-dsf flag for m50
    >
    > BUG=591178
    > TBR=abodenha@chromium.org
    >
    > Committed: https://crrev.com/6ef2be3628f9103ceb708fc256d21956b9ed2eed
    > Cr-Commit-Position: refs/heads/master@{#380818}

To reproduce the failure, you can deploy Chrome synced against this CL and deploy it onto a Samus, then on the DUT do:

# cd /usr/local/telemetry/
# src/tools/perf/run_benchmark --browser=system smoothness.top_25_smooth   

Comment 7 by osh...@chromium.org, May 23 2016

what does this test do? If it's crashing, do you have a stack?
Cc: osh...@chromium.org
Owner: bccheng@chromium.org
Hi Oshima,

The test just iterates through a set of pages to measure smoothness. The failed stack trace is just shown in comment #1.

I will assign the bug back to myself for now as this only fails on Samus/Link/Chell but not other x86 platforms. This could be a compilation bug and I will take another look.

Comment 9 by osh...@chromium.org, May 25 2016

What kind of smoothness it measures and how?
Smoothness while scrolling. More details here: https://www.chromium.org/developers/design-documents/rendering-benchmarks
I wanted to know the chrome stack , as I understand, chrome is crashing?

Thank you for the pointer. I'll take a look and see if I can reproduce the same/similar issue locally.
Sorry for not responding sooner. In the test Chrome is not crashing but the Telemetry harness reported failures on the Python side:


Exception raised when cleaning story run: 

Traceback (most recent call last):
  _RunStoryAndProcessErrorIfNeeded at third_party/catapult/telemetry/telemetry/internal/story_runner.py:110
    test.DidRunPage(state.platform)
  DidRunPage at tools/perf/measurements/smoothness.py:65
    self._tbm.DidRunStory(platform)
  DidRunStory at third_party/catapult/telemetry/telemetry/web_perf/timeline_based_measurement.py:295
    platform.tracing_controller.StopTracing()
  StopTracing at third_party/catapult/telemetry/telemetry/core/tracing_controller.py:39
    return self._tracing_controller_backend.StopTracing()
  StopTracing at third_party/catapult/telemetry/telemetry/internal/platform/tracing_controller_backend.py:124
    '\n'.join(raised_exception_messages))
TracingControllerStoppedError: Exceptions raised when trying to stop tracing:
Traceback (most recent call last):
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/telemetry/internal/platform/tracing_controller_backend.py", line 113, in StopTracing
    agent.StopAgentTracing(builder)
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py", line 143, in StopAgentTracing
    '\n'.join(raised_execption_messages))
ChromeTracingStoppedError: Exceptions raised when trying to stop Chrome devtool tracing:
Error when trying to stop Chrome tracing on devtools at port 60299:
Traceback (most recent call last):
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py", line 128, in StopAgentTracing
    client.StopChromeTracing(trace_data_builder)
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/devtools_client_backend.py", line 354, in StopChromeTracing
    self._tracing_backend.StopTracing(trace_data_builder, timeout)
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/tracing_backend.py", line 146, in StopTracing
    self._CollectTracingData(timeout)
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/tracing_backend.py", line 213, in _CollectTracingData
    traceback.format_exc())
TracingUnrecoverableException: Exception raised while collecting tracing data:
Traceback (most recent call last):
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/tracing_backend.py", line 206, in _CollectTracingData
    self._inspector_websocket.DispatchNotifications(timeout)
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_websocket.py", line 134, in DispatchNotifications
    self._Receive(timeout)
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_websocket.py", line 149, in _Receive
    data = self._socket.recv()
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 596, in recv
    opcode, data = self.recv_data()
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 606, in recv_data
    frame = self.recv_frame()
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 637, in recv_frame
    self._frame_header = self._recv_strict(2)
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 746, in _recv_strict
    bytes = self._recv(shortage)
  File "/usr/local/telemetry/src/third_party/catapult/telemetry/third_party/websocket-client/websocket.py", line 739, in _recv
    raise WebSocketConnectionClosedException()
WebSocketConnectionClosedException

More data points;
- I deployed the Chrome binary compiled for Butterfly to Samus it continues to fail on Samus.
- Samus binary continues to run well on Butterly.

The CL from oshima@ only touches the following function:

diff --git a/content/common/content_switches_internal.cc b/content/common/content_switches_internal.cc
index 5e73d4f3..9200e7f 100644
--- a/content/common/content_switches_internal.cc
+++ b/content/common/content_switches_internal.cc
@@ -25,7 +25,11 @@ static bool g_win32k_renderer_lockdown_disabled = false;
 #endif
 
 bool IsUseZoomForDSFEnabledByDefault() {
+#if defined(OS_CHROMEOS)
+  return true;
+#else
   return false;
+#endif
 }
 
 }  // namespace
Keith mentioned that Samus, Link, and Chell are high DPI devices, and I think that might exactly be the reason for the failure. I added -v -v to the Telemetry command, and it shows that the test is failing due to timeout of waiting for the scrollActionDone action:

(DEBUG) 2016-05-25 02:30:16,183 inspector_websocket._SendRequest:97  sent [{
  "id": 20,
  "method": "Runtime.evaluate",
  "params": {
    "expression": "window.__scrollActionDone",
    "returnByValue": true
  }
}]

IsUseZoomForDSFEnabled is predicated on switches::kEnableUseZoomForDSF:

bool IsUseZoomForDSFEnabled() {
  static bool use_zoom_for_dsf_enabled_by_default =
      IsUseZoomForDSFEnabledByDefault();
  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
  bool enabled =
      (command_line->HasSwitch(switches::kEnableUseZoomForDSF) ||
       use_zoom_for_dsf_enabled_by_default) &&
      command_line->GetSwitchValueASCII(
          switches::kEnableUseZoomForDSF) != "false";

  return enabled;
}

which is enabled when Chrome is started by Telemetry:

(INFO) 2016-05-25 17:30:11,378 cros_browser_backend.Start:104  Restarting Chrome with flags and login
(DEBUG) 2016-05-25 17:30:11,379 cros_interface.GetAllCmdOutput:58  sh -c dbus-send --system --type=method_call --dest=org.chromium.SessionManager /org/chromium/SessionManager org.chromium.SessionManagerInterface.EnableChromeTesting boolean:true array:string:"--enable-gpu-benchmarking,--touch-events=enabled,--running-performance-benchmark,--enable-net-benchmarking,--metrics-recording-only,--no-default-browser-check,--no-first-run,--enable-gpu-benchmarking,--disable-background-networking,--ignore-certificate-errors,--host-resolver-rules=MAP * 127.0.0.1\,EXCLUDE localhost,--testing-fixed-http-port=60259,--testing-fixed-https-port=33768,--user-agent=Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_3) AppleWebKit/537.36 (KHTML\, like Gecko) Chrome/40.0.2194.2 Safari/537.36,--no-proxy-server,--disable-component-extensions-with-background-pages,--disable-default-apps,--enable-smooth-scrolling,--enable-threaded-compositing,--remote-debugging-port=60299,--start-maximized,--ash-disable-system-sounds,--allow-failed-policy-fetch-for-test,--oobe-skip-postlogin,--vmodule=*/chromeos/net/*=2\,*/chromeos/login/*=2,--disable-gaia-services"


The actual scroll distance is injected by the following code, and my guess is the scroll distance is scaled to 0 hence no scrolling done event is seen:

(DEBUG) 2016-05-25 02:30:16,069 inspector_websocket._SendRequest:97  sent [{
  "id": 15,
  "method": "Runtime.evaluate",
  "params": {
    "expression": "// Copyright 2012 The Chromium Authors. All rights reserved.\n// Use of this source code is governed by a BSD-style license that can be\n// found in the LICENSE file.\n\n// This file provides the ScrollAction object, which scrolls a page\n// to the bottom or for a specified distance:\n//   1. var action = new __ScrollAction(callback, opt_distance_func)\n//   2. action.start(scroll_options)\n'use strict';\n\n(function() {\n  var MAX_SCROLL_LENGTH_TIME_MS = 6250;\n\n  function ScrollGestureOptions(opt_options) {\n    if (opt_options) {\n      this.element_ = opt_options.element;\n      this.left_start_ratio_ = opt_options.left_start_ratio;\n      this.top_start_ratio_ = opt_options.top_start_ratio;\n      this.direction_ = opt_options.direction;\n      this.speed_ = opt_options.speed;\n      this.gesture_source_type_ = opt_options.gesture_source_type;\n    } else {\n      this.element_ = document.scrollingElement || document.body;\n      this.left_start_ratio_ = 0.5;\n      this.top_start_ratio_ = 0.5;\n      this.direction_ = 'down';\n      this.speed_ = 800;\n      this.gesture_source_type_ = chrome.gpuBenchmarking.DEFAULT_INPUT;\n    }\n  }\n\n  function supportedByBrowser() {\n    return !!(window.chrome &&\n              chrome.gpuBenchmarking &&\n              chrome.gpuBenchmarking.smoothScrollBy);\n  }\n\n  // This class scrolls a page from the top to the bottom once.\n  //\n  // The page is scrolled down by a single scroll gesture.\n  function ScrollAction(opt_callback, opt_distance_func) {\n    var self = this;\n\n    this.beginMeasuringHook = function() {};\n    this.endMeasuringHook = function() {};\n\n    this.callback_ = opt_callback;\n    this.distance_func_ = opt_distance_func;\n  }\n\n  ScrollAction.prototype.getScrollDistanceDown_ = function() {\n    var clientHeight;\n    // clientHeight is \"special\" for the body element.\n    if (this.element_ == document.body)\n      clientHeight = window.innerHeight;\n    else\n      clientHeight = this.element_.clientHeight;\n\n    return this.element_.scrollHeight -\n           this.element_.scrollTop -\n           clientHeight;\n  };\n\n  ScrollAction.prototype.getScrollDistanceUp_ = function() {\n    return this.element_.scrollTop;\n  };\n\n  ScrollAction.prototype.getScrollDistanceRight_ = function() {\n    var clientWidth;\n    // clientWidth is \"special\" for the body element.\n    if (this.element_ == document.body)\n      clientWidth = window.innerWidth;\n    else\n      clientWidth = this.element_.clientWidth;\n\n    return this.element_.scrollWidth - this.element_.scrollLeft - clientWidth;\n  };\n\n  ScrollAction.prototype.getScrollDistanceLeft_ = function() {\n    return this.element_.scrollLeft;\n  };\n\n  ScrollAction.prototype.getScrollDistance_ = function() {\n    if (this.distance_func_)\n      return this.distance_func_();\n\n    if (this.options_.direction_ == 'down') {\n      return this.getScrollDistanceDown_();\n    } else if (this.options_.direction_ == 'up') {\n      return this.getScrollDistanceUp_();\n    } else if (this.options_.direction_ == 'right') {\n      return this.getScrollDistanceRight_();\n    } else if (this.options_.direction_ == 'left') {\n      return this.getScrollDistanceLeft_();\n    } else if (this.options_.direction_ == 'upleft') {\n      return Math.min(this.getScrollDistanceUp_(),\n                      this.getScrollDistanceLeft_());\n    } else if (this.options_.direction_ == 'upright') {\n      return Math.min(this.getScrollDistanceUp_(),\n                      this.getScrollDistanceRight_());\n    } else if (this.options_.direction_ == 'downleft') {\n      return Math.min(this.getScrollDistanceDown_(),\n                      this.getScrollDistanceLeft_());\n    } else if (this.options_.direction_ == 'downright') {\n      return Math.min(this.getScrollDistanceDown_(),\n                      this.getScrollDistanceRight_());\n    }\n  };\n\n  ScrollAction.prototype.start = function(opt_options) {\n    this.options_ = new ScrollGestureOptions(opt_options);\n    // Assign this.element_ here instead of constructor, because the constructor\n    // ensures this method will be called after the document is loaded.\n    this.element_ = this.options_.element_;\n    requestAnimationFrame(this.startGesture_.bind(this));\n  };\n\n  ScrollAction.prototype.startGesture_ = function() {\n    this.beginMeasuringHook();\n\n    var max_scroll_length_pixels = (MAX_SCROLL_LENGTH_TIME_MS / 1000) *\n        this.options_.speed_;\n    var distance = Math.min(max_scroll_length_pixels,\n                            this.getScrollDistance_());\n\n    var rect = __GestureCommon_GetBoundingVisibleRect(this.options_.element_);\n    var start_left =\n        rect.left + rect.width * this.options_.left_start_ratio_;\n    var start_top =\n        rect.top + rect.height * this.options_.top_start_ratio_;\n    chrome.gpuBenchmarking.smoothScrollBy(\n        distance, this.onGestureComplete_.bind(this), start_left, start_top,\n        this.options_.gesture_source_type_, this.options_.direction_,\n        this.options_.speed_);\n  };\n\n  ScrollAction.prototype.onGestureComplete_ = function() {\n    this.endMeasuringHook();\n\n    // We're done.\n    if (this.callback_)\n      this.callback_();\n  };\n\n  window.__ScrollAction = ScrollAction;\n  window.__ScrollAction_SupportedByBrowser = supportedByBrowser;\n})();\n; 0;",
    "returnByValue": true
  }
}]

Issue 613609 has been merged into this issue.
I have got more information about the failure on Samus. When the test fails, the tab crashed after calling BeginSmoothDrag in ./content/renderer/gpu/gpu_benchmarking_extension.cc:

bool BeginSmoothDrag(v8::Isolate* isolate,
                     float start_x,
                     float start_y,
                     float end_x,
                     float end_y,
                     v8::Local<v8::Function> callback,
                     int gesture_source_type,
                     float speed_in_pixels_s) {
...
}

With Oshima's change, start_y is 690, and prior to that start_y is set at 345.

The actual JS code for scrolling is in ./third_party/catapult/telemetry/telemetry/internal/actions/scroll.js

  ScrollAction.prototype.startGesture_ = function() {
    this.beginMeasuringHook();

    var max_scroll_length_pixels = (MAX_SCROLL_LENGTH_TIME_MS / 1000) *
        this.options_.speed_;
    var distance = Math.min(max_scroll_length_pixels,
                            this.getScrollDistance_());

    var rect = __GestureCommon_GetBoundingVisibleRect(this.options_.element_);
    var start_left =
        rect.left + rect.width * this.options_.left_start_ratio_;
    var start_top =
        rect.top + rect.height * this.options_.top_start_ratio_;
    chrome.gpuBenchmarking.smoothScrollBy(
        distance, this.onGestureComplete_.bind(this), start_left, start_top,
        this.options_.gesture_source_type_, this.options_.direction_,
        this.options_.speed_);
  };

The calculation of 'start_top' is '0 + {690 or 1380} * 0.5' in both cases, respectively. The fields in rect are calculated from calling __GestureCommon_GetBoundingVisibleRect. 

oshima@, could you take a look to see if it makes sense for your DFS change to affect the rect dimensions? Thanks!
Owner: osh...@chromium.org
Hi Oshima, this test is still failing would you be able to look at it ASAP?
With use-zoom-for-dsf mode, the coordinate in blink is native pixels not dip, so that value is expected on 2x DSF device. Where the expectation is defined?
Owner: bccheng@chromium.org
Just had a offline chat with Oshima. The trivial fix should be to divide the pixels by the scale ratio returned from window.devicePixelRatio. I will take a crack at it.
Awesome. Thanks Ben!
Cc: dsinclair@chromium.org
Hi Dan,

I just added you to the bug as we are wondering whether __GestureCommon_GetBoundingVisibleRect should be getting css pixels or absolute pixels. It seems that it should be getting css pixels, but we found once we enable IsUseZoomForDSFEnabledByDefault for some high-end Chromebooks with high DPI screen the values returned by GetBoundingVisibleRect are doubled.

I traced the code and found out that in  Element::getBoundingClientRect it is calling "elementLayoutObject->absoluteQuads(quads);" to get the coordinates. So just want to get your input on whether we should insert some code to convert it back to css pixels along the path somewhere?
Cc: e...@chromium.org
eae@ will probably be better able to answer or redirect to someone who can answer #23.
The actual -> css pixel conversion is definitely happening, as in Element::getBoundingClientRect the width and height for the window are 1265 and 1585, respectively. In the JS code they are reported as 632.5 and 345. I think what's missing is the division of window.devicePixelRatio. Any insight on where this should be performed?
Maybe gpuBenchmarking.visualViewportHeight/Width() returns pixels (dip pixel in old mode, native pixels in new mode) not css pixels?

Comment 27 by e...@chromium.org, Jun 17 2016

Element::getBoundingClientRect should always return CSS pixels. We're changing how we handle high-dpi internally to use the zoom implementation (with IsUseZoomForDSFEnabledByDefault) and during the transition there might be some internal -only APIs that return the wrong value or overcompensate by double-correcting.

Got it. I can confirm that the values returned by Element::getBoundingClientRect are doubled on CrOS devices with high DPI screens with Oshima's CL as mentioned in comment#6:

+++ b/content/common/content_switches_internal.cc
@@ -20,11 +20,14 @@ namespace content {
 namespace {
 
 bool IsUseZoomForDSFEnabledByDefault() {
 #if defined(OS_CHROMEOS)
   return true;
 #else
   return false;
 #endif
 }
 
At the mean time I can prepare a workaround for the Telemetry scrolling test. Could you take this bug and let us know when it is fixed so that we can revert the workaround?
Cc: achuith@chromium.org
Project Member

Comment 31 by bugdroid1@chromium.org, Jun 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6a9801f58fbfb336b69bc8741007d9a6a6c68577

commit 6a9801f58fbfb336b69bc8741007d9a6a6c68577
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Sat Jun 18 19:18:39 2016

Roll src/third_party/catapult/ de3ea6fd8..91953db7a (1 commit).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/de3ea6fd8d96..91953db7ae2b

$ git log de3ea6fd8..91953db7a --date=short --no-merges --format='%ad %ae %s'

BUG= 599656 

TBR=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2080043002
Cr-Commit-Position: refs/heads/master@{#400596}

[modify] https://crrev.com/6a9801f58fbfb336b69bc8741007d9a6a6c68577/DEPS

Cc: rmcilroy@chromium.org conradlo@chromium.org oysteine@chromium.org
I was cc'ed on crbug/621489 and it is very possible (pending bisection bot confirmation) the CSS pixel fix broke the N5x testing:

memory.top_7_stress failure on chromium.perf at Nexus 5X WebView

If that's the case, then I see two action items:

1) We want something like the Nexus 5X testing environment for some Chromebooks too to capture regressions ASAP.
2) Clank seems to have correct implementation to return CSS pixels in getBoundingClientRect. Can someone from the Clank team confirm that?
Blocking: 621489
The bisect failed :/. Could we try a speculative revert (and wait for a catapult roll) to confirm this is the issue?
Cc: klo...@chromium.org
The revert of my CL is uploaded at: https://codereview.chromium.org/2086833003/

Needs a reviewer LGTM.

I also consulted klobag@, and on Android device the value of window.devicePixelRatio is not 1. For example, on my N6P it is 3.5. Apparently Clank managed to have the devicePixelRation to be greater than 1 but getBoundingClientRect still return the CSS pixels 

oshima@, do you know if Clank also have a CL like yours to enable IsUseZoomForDSFEnabledByDefault?
Project Member

Comment 36 by bugdroid1@chromium.org, Jun 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/397a080d90fb52555cf3cdea259a19616a879ebc

commit 397a080d90fb52555cf3cdea259a19616a879ebc
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Tue Jun 28 03:38:46 2016

Roll src/third_party/catapult/ 9f0ffa797..d44648720 (1 commit).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/9f0ffa797de2..d44648720513

$ git log 9f0ffa797..d44648720 --date=short --no-merges --format='%ad %ae %s'

BUG= 599656 

TBR=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2106673002
Cr-Commit-Position: refs/heads/master@{#402389}

[modify] https://crrev.com/397a080d90fb52555cf3cdea259a19616a879ebc/DEPS

Cc: -mshe...@chromium.org
Cc: cn...@chromium.org
Owner: osh...@chromium.org
I found the following places containing code guarded by IsUseZoomForDSFEnabled():

./content/renderer/child_frame_compositing_helper.cc:  if (IsUseZoomForDSFEnabled())
./content/renderer/render_view_browsertest.cc:  if (IsUseZoomForDSFEnabled())
./content/renderer/render_view_browsertest.cc:      IsUseZoomForDSFEnabled() ? 1.f : 2.f;
./content/renderer/render_widget.cc:  if (IsUseZoomForDSFEnabled())
./content/renderer/render_widget.cc:  if (IsUseZoomForDSFEnabled()) {
./content/renderer/render_widget.cc:  if (IsUseZoomForDSFEnabled())
./content/renderer/render_widget.cc:  if (IsUseZoomForDSFEnabled()) {
./content/renderer/render_widget.cc:  if (IsUseZoomForDSFEnabled()) {
./content/renderer/render_view_impl.cc:  if (IsUseZoomForDSFEnabled()) {
./content/renderer/render_view_impl.cc:  if (IsUseZoomForDSFEnabled()) {
./content/common/content_switches_internal.cc:bool IsUseZoomForDSFEnabledByDefault() {
./content/common/content_switches_internal.cc:bool IsUseZoomForDSFEnabled() {
./content/common/content_switches_internal.cc:      IsUseZoomForDSFEnabledByDefault();
./content/browser/frame_host/render_widget_host_view_guest.cc:    if (IsUseZoomForDSFEnabled())
./content/browser/frame_host/render_widget_host_view_guest.cc:    if (IsUseZoomForDSFEnabled())
./content/browser/frame_host/render_widget_host_view_guest.cc:  if (IsUseZoomForDSFEnabled() &&
./content/browser/renderer_host/render_widget_host_impl.cc:  if (IsUseZoomForDSFEnabled())
./content/browser/renderer_host/render_widget_host_view_aura.cc:  if (IsUseZoomForDSFEnabled()) {
./content/shell/renderer/layout_test/blink_test_runner.cc:bool BlinkTestRunner::IsUseZoomForDSFEnabled() {
./content/shell/renderer/layout_test/blink_test_runner.cc:  return content::IsUseZoomForDSFEnabled();
./components/test_runner/test_runner_for_specific_view.cc:      blink::mainThreadIsolate(), delegate()->IsUseZoomForDSFEnabled());

Some of them look look like temporary fixes:
https://cs.chromium.org/chromium/src/content/renderer/child_frame_compositing_helper.cc?q=IsUseZoomForDSFEnabled&sq=package:chromium&dr=C&l=203

Some of the tests are suppressed when DSF is enabled:
https://cs.chromium.org/chromium/src/content/renderer/render_view_browsertest.cc?q=IsUseZoomForDSFEnabled&sq=package:chromium&l=2201&dr=C

Oshima, could you take another look at this? I believe it is caused by fallout from the zoom/DFS scaling factor consolidation. The test can be run from src/tools/perf under the chromium tree:

$./run_benchmark --browser=cros-chrome --remote=<DUT_OF_SAMUS> smoothness.top_25_smooth

Looks like scaling factor should be set to 2 on some path but is currently missing.
Labels: -cros-perf-infra Cros-Perf-Infra
Status: Assigned (was: Started)
Ok I'll look into it.

> Some of them look look like temporary fixes:
> https://cs.chromium.org/chromium/src/content/renderer/child_frame_compositing_helper.cc?q=IsUseZoomForDSFEnabled&sq=package:chromium&dr=C&l=203

This is for guest view, which will be replaced with oopif soon.

> Some of the tests are suppressed when DSF is enabled:
> https://cs.chromium.org/chromium/src/content/renderer/render_view_browsertest.cc?q=IsUseZoomForDSFEnabled&sq=package:chromium&l=2201&dr=C

This simply does not make sense when it's enable.


Blockedon: 631196
Sounds like this is probably failing for the crash described in  issue 631196 
Status: Started (was: Assigned)
Do we have trybots to test a patch?
Project Member

Comment 43 by bugdroid1@chromium.org, Oct 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/927fd323e9ba86831f676db0fc3639eb7c9dba4b

commit 927fd323e9ba86831f676db0fc3639eb7c9dba4b
Author: oshima <oshima@chromium.org>
Date: Tue Oct 04 00:46:52 2016

Return VisualVewport bounds in screen coordinates (DIP) in GpuBenchmarking::VisualViewportXXX method

because telemetry uses it to generate events that are in screen coordinates.

BUG= 599656 
TEST=run smoothness.top_25_smooth test locally and passed.

Review-Url: https://codereview.chromium.org/2373253004
Cr-Commit-Position: refs/heads/master@{#422639}

[modify] https://crrev.com/927fd323e9ba86831f676db0fc3639eb7c9dba4b/content/renderer/gpu/gpu_benchmarking_extension.cc

Status: Fixed (was: Started)
Cc: tdres...@chromium.org charliea@chromium.org bokan@chromium.org eyaich@chromium.org vmi...@chromium.org sullivan@chromium.org brucedaw...@chromium.org dtu@chromium.org bsep@chromium.org lanwei@chromium.org
 Issue 631196  has been merged into this issue.

Comment 46 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 47 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 48 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 49 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 51 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment