Revert "paint timing: Use presentation-time instead of swap-time."

This reverts commit bf746c55.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 566724 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2JmNzQ2YzU1YWRmNDlkMmYyMmNjMzA4MTg1YTE2ZDEyYzRlMTViMDQM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29/72596

Sample Failed Step: not_site_per_process_browser_tests

Original change's description:
> paint timing: Use presentation-time instead of swap-time.
> 
> The presentation-time from gpu gives a more correct view of what the
> user experiences, rather than the swap-time in the renderer process.
> So switch to using the presentation-time to measure the various
> paint-time metrics.
> 
> The various paint-time metrics in telemetry tests (loading benchmark)
> will go up after this change, but it is expected, and it is not a
> regression. Results on various platforms with this change:
> 
> Win 7: https://pinpoint-dot-chromeperf.appspot.com/job/14d09714240000
> Win 10: https://pinpoint-dot-chromeperf.appspot.com/job/149921e4240000
> Mac low-end: https://pinpoint-dot-chromeperf.appspot.com/job/12eeeba5240000
> Android one: https://pinpoint-dot-chromeperf.appspot.com/job/16ab13e9240000
> Android N5x: https://pinpoint-dot-chromeperf.appspot.com/job/12d45da5240000
> 
> BUG=811961
> 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: I70a86eb85e84670942fe3fe6341926b5bc6255b9
> Reviewed-on: https://chromium-review.googlesource.com/1050693
> Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Reviewed-by: Timothy Dresser <tdresser@chromium.org>
> Reviewed-by: Brian Anderson <brianderson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#566724}

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG=811961
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Change-Id: Ia66f54dd90c3010d1103c7ddc657e2765f67a1e3
Reviewed-on: https://chromium-review.googlesource.com/1098775
Cr-Commit-Position: refs/heads/master@{#566743}

cc/trees/layer_tree_host_impl.cc

@@ -135,6 +135,14 @@ bool IsMobileOptimized(LayerTreeImpl* active_tree) {
   return has_fixed_page_scale || has_mobile_viewport;
 }
 
+// Compares two presentation tokens, handling cases where the token
+// wraps around the 32-bit max value.
+bool PresentationTokenGT(uint32_t token1, uint32_t token2) {
+  // There will be underflow in the subtraction if token1 was created
+  // after token2.
+  return (token2 - token1) > 0x80000000u;
+}
+
 viz::ResourceFormat TileRasterBufferFormat(
     const LayerTreeSettings& settings,
     viz::ContextProvider* context_provider,
@@ -1757,7 +1765,7 @@ void LayerTreeHostImpl::DidPresentCompositorFrame(
   std::vector<LayerTreeHost::PresentationTimeCallback> all_callbacks;
   while (!frame_token_infos_.empty()) {
     auto info = frame_token_infos_.begin();
-    if (viz::FrameTokenGT(info->token, frame_token))
+    if (PresentationTokenGT(info->token, frame_token))
       break;
 
     // Update compositor frame latency and smoothness stats only for frames

components/viz/common/quads/compositor_frame_metadata.h

@@ -23,14 +23,6 @@
 
 namespace viz {
 
-// Compares two frame tokens, handling cases where the token wraps around the
-// 32-bit max value.
-inline bool FrameTokenGT(uint32_t token1, uint32_t token2) {
-  // There will be underflow in the subtraction if token1 was created
-  // after token2.
-  return (token2 - token1) > 0x80000000u;
-}
-
 class VIZ_COMMON_EXPORT CompositorFrameMetadata {
  public:
   CompositorFrameMetadata();
@@ -124,11 +116,6 @@ class VIZ_COMMON_EXPORT CompositorFrameMetadata {
 
   // An identifier for the frame. This is used to identify the frame for
   // presentation-feedback, or when the frame-token is sent to the embedder.
-  // For comparing |frame_token| from different frames, use |FrameTokenGT()|
-  // instead of directly comparing them, since the tokens wrap around back to 1
-  // after the 32-bit max value.
-  // TODO(crbug.com/850386): A custom type would be better to avoid incorrect
-  // comparisons.
   uint32_t frame_token = 0;
 
   // Once the display compositor processes a frame with

content/renderer/gpu/render_widget_compositor.cc

@@ -49,7 +49,6 @@
 #include "components/viz/common/frame_sinks/begin_frame_source.h"
 #include "components/viz/common/frame_sinks/copy_output_request.h"
 #include "components/viz/common/frame_sinks/copy_output_result.h"
-#include "components/viz/common/quads/compositor_frame_metadata.h"
 #include "components/viz/common/resources/single_release_callback.h"
 #include "components/viz/common/switches.h"
 #include "content/common/content_switches_internal.h"
@@ -72,7 +71,6 @@
 #include "third_party/blink/public/web/web_selection.h"
 #include "third_party/skia/include/core/SkImage.h"
 #include "ui/base/ui_base_switches.h"
-#include "ui/gfx/presentation_feedback.h"
 #include "ui/gfx/switches.h"
 #include "ui/gl/gl_switches.h"
 #include "ui/native_theme/native_theme_features.h"
@@ -96,51 +94,37 @@ using ReportTimeCallback = blink::WebLayerTreeView::ReportTimeCallback;
 
 class ReportTimeSwapPromise : public cc::SwapPromise {
  public:
-  ReportTimeSwapPromise(ReportTimeCallback callback,
-                        scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-                        base::WeakPtr<RenderWidgetCompositor> compositor);
+  ReportTimeSwapPromise(
+      ReportTimeCallback callback,
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
   ~ReportTimeSwapPromise() override;
 
   void DidActivate() override {}
-  void WillSwap(viz::CompositorFrameMetadata* metadata) override;
+  void WillSwap(viz::CompositorFrameMetadata* metadata) override {}
   void DidSwap() override;
   DidNotSwapAction DidNotSwap(DidNotSwapReason reason) override;
+
   int64_t TraceId() const override;
 
  private:
   ReportTimeCallback callback_;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
-  base::WeakPtr<RenderWidgetCompositor> compositor_;
 
   DISALLOW_COPY_AND_ASSIGN(ReportTimeSwapPromise);
 };
 
 ReportTimeSwapPromise::ReportTimeSwapPromise(
     ReportTimeCallback callback,
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
-    base::WeakPtr<RenderWidgetCompositor> compositor)
-    : callback_(std::move(callback)),
-      task_runner_(std::move(task_runner)),
-      compositor_(compositor) {}
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
+    : callback_(std::move(callback)), task_runner_(std::move(task_runner)) {}
 
 ReportTimeSwapPromise::~ReportTimeSwapPromise() {}
 
-void ReportTimeSwapPromise::WillSwap(viz::CompositorFrameMetadata* metadata) {
-  DCHECK_GT(metadata->frame_token, 0u);
-  metadata->request_presentation_feedback = true;
-  auto* task_runner = task_runner_.get();
-  task_runner->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          &RenderWidgetCompositor::AddPresentationCallback, compositor_,
-          metadata->frame_token,
-          base::BindOnce(std::move(callback_),
-                         blink::WebLayerTreeView::SwapResult::kDidSwap)));
-}
-
 void ReportTimeSwapPromise::DidSwap() {
-  // If swap did happen, then the paint-time will be reported when the
-  // presentation feedback is received.
+  task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(std::move(callback_),
+                                blink::WebLayerTreeView::SwapResult::kDidSwap,
+                                base::TimeTicks::Now()));
 }
 
 cc::SwapPromise::DidNotSwapAction ReportTimeSwapPromise::DidNotSwap(
@@ -1265,22 +1249,6 @@ bool RenderWidgetCompositor::IsForSubframe() {
   return is_for_oopif_;
 }
 
-void RenderWidgetCompositor::DidPresentCompositorFrame(
-    uint32_t frame_token,
-    const gfx::PresentationFeedback& feedback) {
-  DCHECK(layer_tree_host_->GetTaskRunnerProvider()
-             ->MainThreadTaskRunner()
-             ->RunsTasksInCurrentSequence());
-  while (!presentation_callbacks_.empty()) {
-    const auto& front = presentation_callbacks_.begin();
-    if (viz::FrameTokenGT(front->first, frame_token))
-      break;
-    for (auto& callback : front->second)
-      std::move(callback).Run(feedback.timestamp);
-    presentation_callbacks_.erase(front);
-  }
-}
-
 void RenderWidgetCompositor::RequestScheduleAnimation() {
   delegate_->RequestScheduleAnimation();
 }
@@ -1310,8 +1278,7 @@ void RenderWidgetCompositor::SetContentSourceId(uint32_t id) {
 void RenderWidgetCompositor::NotifySwapTime(ReportTimeCallback callback) {
   QueueSwapPromise(std::make_unique<ReportTimeSwapPromise>(
       std::move(callback),
-      layer_tree_host_->GetTaskRunnerProvider()->MainThreadTaskRunner(),
-      weak_factory_.GetWeakPtr()));
+      layer_tree_host_->GetTaskRunnerProvider()->MainThreadTaskRunner()));
 }
 
 void RenderWidgetCompositor::RequestBeginMainFrameNotExpected(bool new_state) {
@@ -1328,25 +1295,6 @@ void RenderWidgetCompositor::CreateRenderFrameObserver(
       std::move(render_frame_metadata_observer));
 }
 
-void RenderWidgetCompositor::AddPresentationCallback(
-    uint32_t frame_token,
-    base::OnceCallback<void(base::TimeTicks)> callback) {
-  if (!presentation_callbacks_.empty()) {
-    auto& previous = presentation_callbacks_.back();
-    uint32_t previous_frame_token = previous.first;
-    if (previous_frame_token == frame_token) {
-      previous.second.push_back(std::move(callback));
-      DCHECK_LE(previous.second.size(), 25u);
-      return;
-    }
-    DCHECK(viz::FrameTokenGT(frame_token, previous_frame_token));
-  }
-  std::vector<base::OnceCallback<void(base::TimeTicks)>> callbacks;
-  callbacks.push_back(std::move(callback));
-  presentation_callbacks_.push_back({frame_token, std::move(callbacks)});
-  DCHECK_LE(presentation_callbacks_.size(), 25u);
-}
-
 void RenderWidgetCompositor::SetURLForUkm(const GURL& url) {
   layer_tree_host_->SetURLForUkm(url);
 }

content/renderer/gpu/render_widget_compositor.h

@@ -8,7 +8,6 @@
 #include <stdint.h>
 
 #include "base/callback.h"
-#include "base/containers/circular_deque.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "base/values.h"
@@ -209,10 +208,10 @@ class CONTENT_EXPORT RenderWidgetCompositor
   void DidCommitAndDrawFrame() override;
   void DidReceiveCompositorFrameAck() override;
   void DidCompletePageScaleAnimation() override;
-  bool IsForSubframe() override;
   void DidPresentCompositorFrame(
       uint32_t frame_token,
-      const gfx::PresentationFeedback& feedback) override;
+      const gfx::PresentationFeedback& feedback) override {}
+  bool IsForSubframe() override;
 
   // cc::LayerTreeHostSingleThreadClient implementation.
   void RequestScheduleAnimation() override;
@@ -230,10 +229,6 @@ class CONTENT_EXPORT RenderWidgetCompositor
       mojom::RenderFrameMetadataObserverRequest request,
       mojom::RenderFrameMetadataObserverClientPtrInfo client_info);
 
-  void AddPresentationCallback(
-      uint32_t frame_token,
-      base::OnceCallback<void(base::TimeTicks)> callback);
-
   cc::LayerTreeHost* layer_tree_host() { return layer_tree_host_.get(); }
 
  protected:
@@ -264,10 +259,6 @@ class CONTENT_EXPORT RenderWidgetCompositor
   base::OnceClosure layout_and_paint_async_callback_;
 
   viz::FrameSinkId frame_sink_id_;
-  base::circular_deque<
-      std::pair<uint32_t,
-                std::vector<base::OnceCallback<void(base::TimeTicks)>>>>
-      presentation_callbacks_;
 
   base::WeakPtrFactory<RenderWidgetCompositor> weak_factory_;