cc: Fix size_t to int truncations in tiles/ and trees/

cc/tiles/tile_manager.cc

 // Copyright 2012 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/tiles/tile_manager.h"
 
 #include <algorithm>
 #include <limits>
 #include <string>
 
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "cc/base/histograms.h"
 #include "cc/debug/devtools_instrumentation.h"
 #include "cc/debug/frame_viewer_instrumentation.h"
 #include "cc/debug/traced_value.h"
 #include "cc/layers/picture_layer_impl.h"
 #include "cc/raster/raster_buffer.h"
 #include "cc/raster/tile_task_runner.h"
 #include "cc/tiles/tile.h"
 #include "ui/gfx/geometry/rect_conversions.h"
@@ skipping 176 matching lines @@
 
 }  // namespace
 
 RasterTaskCompletionStats::RasterTaskCompletionStats()
     : completed_count(0u), canceled_count(0u) {}
 
 scoped_refptr<base::trace_event::ConvertableToTraceFormat>
 RasterTaskCompletionStatsAsValue(const RasterTaskCompletionStats& stats) {
   scoped_refptr<base::trace_event::TracedValue> state =
       new base::trace_event::TracedValue();
-  state->SetInteger("completed_count", stats.completed_count);
-  state->SetInteger("canceled_count", stats.canceled_count);
+  state->SetInteger("completed_count",
+                    base::saturated_cast<int>(stats.completed_count));
+  state->SetInteger("canceled_count",
+                    base::saturated_cast<int>(stats.canceled_count));
   return state;
 }
 
 // static
 scoped_ptr<TileManager> TileManager::Create(
     TileManagerClient* client,
     base::SequencedTaskRunner* task_runner,
     ResourcePool* resource_pool,
     TileTaskRunner* tile_task_runner,
     size_t scheduled_raster_task_limit) {
@@ skipping 183 matching lines @@
 scoped_refptr<base::trace_event::ConvertableToTraceFormat>
 TileManager::BasicStateAsValue() const {
   scoped_refptr<base::trace_event::TracedValue> value =
       new base::trace_event::TracedValue();
   BasicStateAsValueInto(value.get());
   return value;
 }
 
 void TileManager::BasicStateAsValueInto(
     base::trace_event::TracedValue* state) const {
-  state->SetInteger("tile_count", tiles_.size());
+  state->SetInteger("tile_count", base::saturated_cast<int>(tiles_.size()));
   state->SetBoolean("did_oom_on_last_assign", did_oom_on_last_assign_);
   state->BeginDictionary("global_state");
   global_state_.AsValueInto(state);
   state->EndDictionary();
 }
 
 scoped_ptr<EvictionTilePriorityQueue>
 TileManager::FreeTileResourcesUntilUsageIsWithinLimit(
     scoped_ptr<EvictionTilePriorityQueue> eviction_priority_queue,
     const MemoryUsage& limit,
@@ skipping 532 matching lines @@
     client_->NotifyTileStateChanged(tile);
   }
 
   DCHECK(IsReadyToActivate());
   ready_to_activate_check_notifier_.Schedule();
 }
 
 TileManager::MemoryUsage::MemoryUsage() : memory_bytes_(0), resource_count_(0) {
 }
 
-TileManager::MemoryUsage::MemoryUsage(int64 memory_bytes, int resource_count)
-    : memory_bytes_(memory_bytes), resource_count_(resource_count) {
+TileManager::MemoryUsage::MemoryUsage(size_t memory_bytes,
+                                      size_t resource_count)
+    : memory_bytes_(static_cast<int64>(memory_bytes)),
+      resource_count_(static_cast<int>(resource_count)) {
+  // MemoryUsage is constructed using size_ts, since it deals with memory and
+  // the inputs are typically size_t. However, during the course of usage (in
+  // particular operator-=) can cause internal values to become negative. Thus,

[danakj, 2015/06/09 17:39:49]

Can we DCHECK that they won't become negative? Is it intended?

[vmpstr, 2015/06/09 18:37:17]

Yeah the size going negative is somewhat intentional. We can subtract a memory
usage that is larger that this.

So we can do a -= b; where b > a, in that case a will become negative. This just
means that a.Exceeds(c) will be false for any regularly formed c. 

I feel like this is just an implementation detail of this class. The alternative
would be to have a check in operator-= to make sure it doesn't go below 0. Do
you think that's preferable?

[danakj, 2015/06/09 19:05:47]

Ya lets just set to 0 if it would become negative.

+  // member variables are signed.
+  DCHECK_LE(memory_bytes,
+            static_cast<size_t>(std::numeric_limits<int64>::max()));
+  DCHECK_LE(resource_count,
+            static_cast<size_t>(std::numeric_limits<int>::max()));
 }
 
 // static
 TileManager::MemoryUsage TileManager::MemoryUsage::FromConfig(
     const gfx::Size& size,
     ResourceFormat format) {
   // We can use UncheckedMemorySizeBytes here since this is used with a tile
   // size which is determined by the compositor (it's at most max texture size).
   return MemoryUsage(Resource::UncheckedMemorySizeBytes(size, format), 1);
 }
@@ skipping 28 matching lines @@
   result -= other;
   return result;
 }
 
 bool TileManager::MemoryUsage::Exceeds(const MemoryUsage& limit) const {
   return memory_bytes_ > limit.memory_bytes_ ||
          resource_count_ > limit.resource_count_;
 }
 
 }  // namespace cc