Make scroll offset type of float in cc

cc/trees/layer_tree_host.cc

 // Copyright 2011 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.
 
 #include "cc/trees/layer_tree_host.h"
 
 #include <algorithm>
 #include <stack>
 #include <string>
 
@@ skipping 22 matching lines @@
 #include "cc/resources/prioritized_resource_manager.h"
 #include "cc/resources/ui_resource_request.h"
 #include "cc/trees/layer_tree_host_client.h"
 #include "cc/trees/layer_tree_host_common.h"
 #include "cc/trees/layer_tree_host_impl.h"
 #include "cc/trees/layer_tree_impl.h"
 #include "cc/trees/occlusion_tracker.h"
 #include "cc/trees/single_thread_proxy.h"
 #include "cc/trees/thread_proxy.h"
 #include "cc/trees/tree_synchronizer.h"
+#include "ui/gfx/geometry/vector2d_conversions.h"
 #include "ui/gfx/size_conversions.h"
 
 namespace {
 static base::StaticAtomicSequenceNumber s_layer_tree_host_sequence_number;
 }
 
 namespace cc {
 
 RendererCapabilities::RendererCapabilities(ResourceFormat best_texture_format,
                                            bool allow_partial_texture_updates,
@@ skipping 1006 matching lines @@
   ScopedPtrVector<SwapPromise>::iterator it = info->swap_promises.begin();
   for (; it != info->swap_promises.end(); ++it) {
     scoped_ptr<SwapPromise> swap_promise(info->swap_promises.take(it));
     TRACE_EVENT_FLOW_STEP0("input",
                            "LatencyInfo.Flow",
                            TRACE_ID_DONT_MANGLE(swap_promise->TraceId()),
                            "Main thread scroll update");
     QueueSwapPromise(swap_promise.Pass());
   }
 
-  gfx::Vector2d inner_viewport_scroll_delta;
-  gfx::Vector2d outer_viewport_scroll_delta;
+  gfx::Vector2dF inner_viewport_scroll_delta;
+  gfx::Vector2dF outer_viewport_scroll_delta;
 
   for (size_t i = 0; i < info->scrolls.size(); ++i) {
     Layer* layer = LayerTreeHostCommon::FindLayerInSubtree(
         root_layer_.get(), info->scrolls[i].layer_id);
     if (!layer)
       continue;
     if (layer == outer_viewport_scroll_layer_.get()) {
       outer_viewport_scroll_delta += info->scrolls[i].scroll_delta;
     } else if (layer == inner_viewport_scroll_layer_.get()) {
       inner_viewport_scroll_delta += info->scrolls[i].scroll_delta;
     } else {
-      layer->SetScrollOffsetFromImplSide(layer->scroll_offset() +
-                                         info->scrolls[i].scroll_delta);
+      layer->SetScrollOffsetFromImplSide(
+          layer->scroll_offset() +
+          gfx::ScrollOffset(info->scrolls[i].scroll_delta));
     }
   }
 
   if (!inner_viewport_scroll_delta.IsZero() ||
       !outer_viewport_scroll_delta.IsZero() ||
       info->page_scale_delta != 1.f ||
       info->top_controls_delta) {
     // SetScrollOffsetFromImplSide above could have destroyed the tree,
     // so re-get this layer before doing anything to it.
 
     // Preemptively apply the scroll offset and scale delta here before sending
     // it to the client.  If the client comes back and sets it to the same
     // value, then the layer can early out without needing a full commit.
     DCHECK(inner_viewport_scroll_layer_.get());  // We should always have this.
 
     inner_viewport_scroll_layer_->SetScrollOffsetFromImplSide(
         inner_viewport_scroll_layer_->scroll_offset() +
-        inner_viewport_scroll_delta);
+        gfx::ScrollOffset(inner_viewport_scroll_delta));;

[danakj, 2014/09/25 22:01:26]

double;;

[Yufeng Shen (Slow to review), 2014/09/26 20:19:07]

Done.

     if (outer_viewport_scroll_layer_.get()) {
       outer_viewport_scroll_layer_->SetScrollOffsetFromImplSide(
           outer_viewport_scroll_layer_->scroll_offset() +
-          outer_viewport_scroll_delta);
+          gfx::ScrollOffset(outer_viewport_scroll_delta));

[danakj, 2014/09/25 22:01:26]

Throwing this out as an idea, idk if it's a good one. I see lots of 

offset + gfx::ScrollOffset(delta)

we could have a

gfx::ScrollOffset ScrollOffsetWithDelta(offset, delta) that did this. It would
actually make a bit more characters at call sites, though. Would it be more
clear? What do you think?

[Yufeng Shen (Slow to review), 2014/09/25 22:15:01]

what do you think if I have an overload

ScrollOffset operator+(const ScrollOffset& lhs, const Vector2dF& rhs) ?
ScrollOffset operator+(const ScrollOffset& lhs, const Vector2d& rhs)

there are a lot of calls in the unittest files that have this 
offset + delta or offset + delta + delta pattern
having these overload will make the change much simpler.

     }
     ApplyPageScaleDeltaFromImplSide(info->page_scale_delta);
 
-    client_->ApplyViewportDeltas(
-        inner_viewport_scroll_delta + outer_viewport_scroll_delta,
-        info->page_scale_delta,
-        info->top_controls_delta);
+    gfx::Vector2dF viewport_scroll_delta =
+        inner_viewport_scroll_delta + outer_viewport_scroll_delta;
+
+    client_->ApplyViewportDeltas(gfx::ToFlooredVector2d(viewport_scroll_delta),

[danakj, 2014/09/25 22:01:26]

why flooring here? not sure why you changed anything here

[Yufeng Shen (Slow to review), 2014/09/26 20:19:07]

Done.

+                                 info->page_scale_delta,
+                                 info->top_controls_delta);
   }
 }
 
 void LayerTreeHost::StartRateLimiter() {
   if (inside_begin_main_frame_)
     return;
 
   if (!rate_limit_timer_.IsRunning()) {
     rate_limit_timer_.Start(FROM_HERE,
                             base::TimeDelta(),
@@ skipping 194 matching lines @@
   swap_promise_list_.push_back(swap_promise.Pass());
 }
 
 void LayerTreeHost::BreakSwapPromises(SwapPromise::DidNotSwapReason reason) {
   for (size_t i = 0; i < swap_promise_list_.size(); i++)
     swap_promise_list_[i]->DidNotSwap(reason);
   swap_promise_list_.clear();
 }
 
 }  // namespace cc