Revert to old rect conversion behavior

Revert to old rect conversion behavior

With https://chromiumcodereview.appspot.com/10996037, some rect conversion behaviors changed and regressions were introduced. To ensure that no more appear, this CL reverts the behavior changes introduced by the previous patch.

The old behavior was to convert from RectF to Rect by flooring the position and size. Ideally, this old behavior would not appear in the public API to discourage its use, but the legacy behavior is required in enough places that it seems worse to duplicate the ToFlooredRect function at each call site. Instead, I've named the function ToFlooredRectDeprecated to discourage its use in new code. I've included unit tests to ensure that this function replicates the old behavior.

BUG=None
TEST=RectTest.ToFlooredRect


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=164383

[sky, 2012-10-22 15:08:37]

https://codereview.chromium.org/11235017/diff/9/ui/gfx/rect_conversions.h
File ui/gfx/rect_conversions.h (right):

https://codereview.chromium.org/11235017/diff/9/ui/gfx/rect_conversions.h#new...
ui/gfx/rect_conversions.h:21: UI_EXPORT Rect ToFlooredRectDeprecated(const
RectF& rect);
If we're going to keep this around, it should not be called deprecated.

[Ian Vollick, 2012-10-22 15:13:43]

On 2012/10/22 15:08:37, sky wrote:
> https://codereview.chromium.org/11235017/diff/9/ui/gfx/rect_conversions.h
> File ui/gfx/rect_conversions.h (right):
> 
>
https://codereview.chromium.org/11235017/diff/9/ui/gfx/rect_conversions.h#new...
> ui/gfx/rect_conversions.h:21: UI_EXPORT Rect ToFlooredRectDeprecated(const
> RectF& rect);
> If we're going to keep this around, it should not be called deprecated.

It's true. If this patch were to land, my plan was to open a bug (or bugs) to
track the migration from ToFlooredRectDeprecated to enclosed or enclosing, as
appropriate. Eventually, this method should disappear, but this would ensure
that we don't experience regressions while these decisions are being made.

[danakj, 2012-10-22 15:13:46]

https://codereview.chromium.org/11235017/diff/9/ui/gfx/rect_conversions.h
File ui/gfx/rect_conversions.h (right):

https://codereview.chromium.org/11235017/diff/9/ui/gfx/rect_conversions.h#new...
ui/gfx/rect_conversions.h:21: UI_EXPORT Rect ToFlooredRectDeprecated(const
RectF& rect);
On 2012/10/22 15:08:37, sky wrote:
> If we're going to keep this around, it should not be called deprecated.

It's funny, there was a thread on chromium-dev about deprecating functions
yesterday, ("cross-platform macro to mark a function or class as deprecated")
and a few people thought we should put "deprecated" in the function name in
those cases for reviewers to see that people are using something that should be
avoided.

[sky, 2012-10-22 15:44:22]

As long as the plan is for these to be removed, LGTM

[commit-bot: I haz the power, 2012-10-22 19:56:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/11235017/9

[commit-bot: I haz the power, 2012-10-22 19:56:10]

Presubmit check for 11235017-9 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/renderer

Presubmit checks took 1.1s to calculate.

[jamesr, 2012-10-22 19:57:54]

lgtm

[commit-bot: I haz the power, 2012-10-22 20:54:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/11235017/9

[commit-bot: I haz the power, 2012-10-24 13:22:50]

Retried try job too often for step(s) base_unittests, browser_tests,
cacheinvalidation_unittests, check_deps, chrome_frame_net_tests,
chrome_frame_unittests, content_browsertests, content_unittests,
crypto_unittests, gpu_unittests, installer_util_unittests, interactive_ui_tests,
ipc_tests, jingle_unittests, media_unittests, mini_installer_test,
nacl_integration, net_unittests, printing_unittests, remoting_unittests,
safe_browsing_tests, sql_unittests, sync_integration_tests, sync_unit_tests,
unit_tests

[commit-bot: I haz the power, 2012-10-26 13:33:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/11235017/9

[commit-bot: I haz the power, 2012-10-26 13:33:21]

Failed to apply patch for content/browser/renderer_host/backing_store_mac.mm:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/browser/renderer_host/backing_store_mac.mm
  Hunk #2 FAILED at 91.
  Hunk #3 FAILED at 111.
  2 out of 3 hunks FAILED -- saving rejects to file
content/browser/renderer_host/backing_store_mac.mm.rej

Patch:       content/browser/renderer_host/backing_store_mac.mm
Index: content/browser/renderer_host/backing_store_mac.mm
diff --git a/content/browser/renderer_host/backing_store_mac.mm
b/content/browser/renderer_host/backing_store_mac.mm
index
0cb5a3b6c2f7d2976b0b3e36812cf17a374466c3..e154d014c321cd442d9640368fffa630c5eeab6f
100644
--- a/content/browser/renderer_host/backing_store_mac.mm
+++ b/content/browser/renderer_host/backing_store_mac.mm
@@ -18,22 +18,11 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "ui/gfx/rect.h"
+#include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/size_conversions.h"
 #include "ui/gfx/scoped_cg_context_save_gstate_mac.h"
 #include "ui/surface/transport_dib.h"
 
-namespace {
-
-// Returns a Rect obtained by flooring the values of the given RectF.
-gfx::Rect ToFlooredRect(const gfx::RectF& rect) {
-  return gfx::Rect(static_cast<int>(std::floor(rect.x())),
-                   static_cast<int>(std::floor(rect.y())),
-                   static_cast<int>(std::floor(rect.width())),
-                   static_cast<int>(std::floor(rect.height())));
-}
-
-} // namespace
-
 namespace content {
 
 // Mac Backing Stores:
@@ -102,7 +91,7 @@ void BackingStoreMac::PaintToBackingStore(
   gfx::Size pixel_size = gfx::ToFlooredSize(
       size().Scale(device_scale_factor_));
   gfx::Rect pixel_bitmap_rect =
-      ToFlooredRect(bitmap_rect.Scale(scale_factor));
+      ToFlooredRectDeprecated(bitmap_rect.Scale(scale_factor));
 
   size_t bitmap_byte_count =
       pixel_bitmap_rect.width() * pixel_bitmap_rect.height() * 4;
@@ -122,7 +111,7 @@ void BackingStoreMac::PaintToBackingStore(
   for (size_t i = 0; i < copy_rects.size(); i++) {
     const gfx::Rect& copy_rect = copy_rects[i];
     gfx::Rect pixel_copy_rect =
-        ToFlooredRect(copy_rect.Scale(scale_factor));
+        ToFlooredRectDeprecated(copy_rect.Scale(scale_factor));
 
     // Only the subpixels given by copy_rect have pixels to copy.
     base::mac::ScopedCFTypeRef<CGImageRef> image(

[commit-bot: I haz the power, 2012-10-26 13:56:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/11235017/20001

[commit-bot: I haz the power, 2012-10-26 19:48:45]

Change committed as 164383