Create cc::TilingData based on WebCore::TilingData.

Create cc::TilingData based on WebCore::TilingData.

This is a direct port of the TilingData class from the WebKit repo, found in:
Source/WebCore/platform/graphics/gpu/TilingData.h|cpp

Previous commits from non-google folks have been:
- ossy replacing the non-copyable macro (which is not present here).
- hyatt replacing right()/bottom() with maxX()/maxY() (we use right()/bottom() again here, with gfx::Rect instead of IntRect).
- mitz using ASSERT_UNUSED instead of ASSERT guarded on NDEBUG (we use DCHECK instead here).

Covered by existing tests; this replaces the use of WebCore::TilingData with the new cc::TilingData.

R=enne
BUG=147395
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=165681

[danakj, 2012-11-02 00:14:58]

:D

[jamesr, 2012-11-02 00:18:21]

lgtm

these patches make me uncomfortably excited

[tfarina, 2012-11-02 00:18:46]

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h
File cc/tiling_data.h (right):

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h#newcode24
cc/tiling_data.h:24: gfx::Size total_size() const { return total_size_; }
const-ref as well.

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h#newcode25
cc/tiling_data.h:25: void SetTotalSize(const gfx::Size total_size);
const-ref!

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h#newcode28
cc/tiling_data.h:28: void SetMaxTextureSize(gfx::Size max_texture_size);
const-ref!

Same at the ctor!

[tfarina, 2012-11-02 00:19:07]

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h
File cc/tiling_data.h (right):

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h#newcode1
cc/tiling_data.h:1: // Copyright 2010 The Chromium Authors. All rights reserved.
2010 -> 2012?

[jamesr, 2012-11-02 00:24:23]

On 2012/11/02 00:18:46, tfarina wrote:
> https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h
> File cc/tiling_data.h (right):
> 
> https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h#newcode24
> cc/tiling_data.h:24: gfx::Size total_size() const { return total_size_; }
> const-ref as well.

If it's a struct with 2 ints (as this one is), you should pass by value.

[tfarina, 2012-11-02 00:25:45]

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.cc
File cc/tiling_data.cc (right):

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.cc#newcode1
cc/tiling_data.cc:1: // Copyright 2010 The Chromium Authors. All rights
reserved.
2010 -> 2012?

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.cc#newcode5
cc/tiling_data.cc:5: #include "config.h"
what is this for?

[danakj, 2012-11-02 00:28:03]

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.cc
File cc/tiling_data.cc (right):

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.cc#newcode1
cc/tiling_data.cc:1: // Copyright 2010 The Chromium Authors. All rights
reserved.
On 2012/11/02 00:25:45, tfarina wrote:
> 2010 -> 2012?

Keeping the year from the old file, as this is a copy/change of that.

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.cc#newcode5
cc/tiling_data.cc:5: #include "config.h"
On 2012/11/02 00:25:45, tfarina wrote:
> what is this for?

same as all the other .cc in cc/ until we are clear of webcore.

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h
File cc/tiling_data.h (right):

https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h#newcode24
cc/tiling_data.h:24: gfx::Size total_size() const { return total_size_; }
On 2012/11/02 00:18:46, tfarina wrote:
> const-ref as well.

no const ref for <= 4 ints :)

[commit-bot: I haz the power, 2012-11-02 00:28:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/11359030/1

[tfarina, 2012-11-02 00:30:36]

On 2012/11/02 00:28:03, danakj wrote:
>https://codereview.chromium.org/11359030/diff/1/cc/tiling_data.h#newcode24
> cc/tiling_data.h:24: gfx::Size total_size() const { return total_size_; }
> On 2012/11/02 00:18:46, tfarina wrote:
> > const-ref as well.
> 
> no const ref for <= 4 ints :)

No, this is really inconsistent with our code base.

$ g gs "const gfx::Size&" | grep return | wc -l                                 
                                                                                
                            
31

$ g gs "gfx::Size .*" | grep return | wc -l                                     
                                                                                
                  
20

$ g gs "const gfx::Size&" | wc -l
1083

$ g gs "const gfx::Size " | wc -l                                               
                                                                                
                              
81

[jamesr, 2012-11-02 00:31:48]

Probably because people are mechanically using const ref for tiny structs
without thinking what it means, and/or telling other people to do it.  It's a
bad practice and we shouldn't encourage it.

[tfarina, 2012-11-02 00:36:29]

On Thu, Nov 1, 2012 at 10:31 PM,  <jamesr@chromium.org> wrote:
> Probably because people are mechanically using const ref for tiny structs
> without thinking what it means, and/or telling other people to do it.  It's
> a
> bad practice and we shouldn't encourage it.
>
Something new to me :)

$ g grep -n ".*gfx::Size&.*" cc/ | wc -l
109

Is it worth fixing this in cc/?



-- 
Thiago

[jamesr1, 2012-11-02 00:39:37]

On Thu, Nov 1, 2012 at 5:36 PM, Thiago Farina <tfarina@chromium.org> wrote:

> On Thu, Nov 1, 2012 at 10:31 PM,  <jamesr@chromium.org> wrote:
> > Probably because people are mechanically using const ref for tiny structs
> > without thinking what it means, and/or telling other people to do it.
>  It's
> > a
> > bad practice and we shouldn't encourage it.
> >
> Something new to me :)
>
> $ g grep -n ".*gfx::Size&.*" cc/ | wc -l
> 109
>
> Is it worth fixing this in cc/?
>
>
I wouldn't worry about changing existing code.

- James


>
>
> --
> Thiago
>

[commit-bot: I haz the power, 2012-11-02 05:03:42]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2012-11-02 15:32:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/11359030/1

[commit-bot: I haz the power, 2012-11-02 15:33:02]

Failed to apply patch for cc/cc.gyp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/cc.gyp
  Hunk #1 FAILED at 218.
  Hunk #2 succeeded at 263 with fuzz 1 (offset -2 lines).
  Hunk #3 succeeded at 274 with fuzz 2 (offset -3 lines).
  1 out of 3 hunks FAILED -- saving rejects to file cc/cc.gyp.rej

Patch:       cc/cc.gyp
Index: cc/cc.gyp
diff --git a/cc/cc.gyp b/cc/cc.gyp
index
8a6e93b4e91270ebfb9c09e142b93f7797745a4d..cae80432e4350b14e432c482947477b288133f4b
100644
--- a/cc/cc.gyp
+++ b/cc/cc.gyp
@@ -218,6 +218,8 @@
       'texture_uploader.h',
       'tiled_layer.cc',
       'tiled_layer.h',
+      'tiling_data.cc',
+      'tiling_data.h',
       'tree_synchronizer.cc',
       'tree_synchronizer.h',
       'video_layer.cc',
@@ -265,7 +267,6 @@
         'stubs/NotImplemented.h',
         'stubs/Region.h',
         'stubs/SkiaUtils.h',
-        'stubs/TilingData.h',
         'stubs/UnitBezier.h',
 
         'stubs/config.h',
@@ -278,7 +279,6 @@
         'stubs/int_size.h',
         'stubs/not_implemented.h',
         'stubs/skia_utils.h',
-        'stubs/tiling_data.h',
         'stubs/unit_bezier.h',
       ],
     },