cc: Fix build issues for adding ‘chromium_code’: 1 to cc.gyp and cc_tests.gyp

cc: Fix build issues for adding ‘chromium_code’: 1 to cc.gyp and cc_tests.gyp

The flag flip will be a follow up once ChromeOS build is sorted out.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=191417

[jamesr, 2013-03-29 00:07:16]

Didn't have a chance to go through the whole patch yet, but spotted a few
things.

https://codereview.chromium.org/13206004/diff/1/cc/base/tiling_data_unittest.cc
File cc/base/tiling_data_unittest.cc (right):

https://codereview.chromium.org/13206004/diff/1/cc/base/tiling_data_unittest....
cc/base/tiling_data_unittest.cc:1066: EXPECT_EQ(static_cast<size_t>(num_tiles),
expected.size());
num_tiles is a size_t, according to the function signature. why is this cast
needed?

https://codereview.chromium.org/13206004/diff/1/cc/base/worker_pool.cc
File cc/base/worker_pool.cc (right):

https://codereview.chromium.org/13206004/diff/1/cc/base/worker_pool.cc#newcod...
cc/base/worker_pool.cc:194: DCHECK(pending_tasks_.empty());
one slight bummer about this change is if the DCHECK fails this won't print out
the actual size. can you change it to DCHECK_EQ(..., 0u)?

https://codereview.chromium.org/13206004/diff/1/cc/base/worker_pool.cc#newcod...
cc/base/worker_pool.cc:433: DCHECK(completed_tasks_.empty());
ditto

https://codereview.chromium.org/13206004/diff/1/cc/cc.gyp
File cc/cc.gyp (right):

https://codereview.chromium.org/13206004/diff/1/cc/cc.gyp#newcode349
cc/cc.gyp:349: 'chromium_code': 1,
nit: could you put this at the top of the variables section/

https://codereview.chromium.org/13206004/diff/1/cc/cc_tests.gyp
File cc/cc_tests.gyp (right):

https://codereview.chromium.org/13206004/diff/1/cc/cc_tests.gyp#newcode147
cc/cc_tests.gyp:147: 'chromium_code': 1,
we already have 'chromium_code': 0 at the top of this variables block, could you
just modify that instead?

[danakj, 2013-03-29 00:17:35]

https://codereview.chromium.org/13206004/diff/1/cc/base/tiling_data_unittest.cc
File cc/base/tiling_data_unittest.cc (right):

https://codereview.chromium.org/13206004/diff/1/cc/base/tiling_data_unittest....
cc/base/tiling_data_unittest.cc:1066: EXPECT_EQ(static_cast<size_t>(num_tiles),
expected.size());
On 2013/03/29 00:07:16, jamesr wrote:
> num_tiles is a size_t, according to the function signature. why is this cast
> needed?

Oh.. hm.. it's not. removed.

https://codereview.chromium.org/13206004/diff/1/cc/base/worker_pool.cc
File cc/base/worker_pool.cc (right):

https://codereview.chromium.org/13206004/diff/1/cc/base/worker_pool.cc#newcod...
cc/base/worker_pool.cc:194: DCHECK(pending_tasks_.empty());
On 2013/03/29 00:07:16, jamesr wrote:
> one slight bummer about this change is if the DCHECK fails this won't print
out
> the actual size. can you change it to DCHECK_EQ(..., 0u)?

True.. okay I'll switch all these to == 0

https://codereview.chromium.org/13206004/diff/1/cc/cc.gyp
File cc/cc.gyp (right):

https://codereview.chromium.org/13206004/diff/1/cc/cc.gyp#newcode349
cc/cc.gyp:349: 'chromium_code': 1,
On 2013/03/29 00:07:16, jamesr wrote:
> nit: could you put this at the top of the variables section/

Done.

https://codereview.chromium.org/13206004/diff/1/cc/cc_tests.gyp
File cc/cc_tests.gyp (right):

https://codereview.chromium.org/13206004/diff/1/cc/cc_tests.gyp#newcode147
cc/cc_tests.gyp:147: 'chromium_code': 1,
On 2013/03/29 00:07:16, jamesr wrote:
> we already have 'chromium_code': 0 at the top of this variables block, could
you
> just modify that instead?

heh, whoops.

[danakj, 2013-03-29 00:19:16]

Uploaded PTAL

[jamesr, 2013-03-29 00:29:45]

lgtm

[commit-bot: I haz the power, 2013-03-29 00:41:30]

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

[danakj, 2013-03-29 00:48:37]

Weird, android compiler caught a few more things that linux clang didn't.

[commit-bot: I haz the power, 2013-03-29 00:50:56]

Failed to trigger a try job on linux_clang
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-03-29 00:55:47]

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

[commit-bot: I haz the power, 2013-03-29 01:20:22]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[danakj, 2013-03-29 01:20:42]

Fixed (hopefully) all the windows stuff with !! and != 0 and > 0, if you'd like
to look.

[commit-bot: I haz the power, 2013-03-29 01:23:24]

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

[danakj, 2013-03-29 01:25:38]

The rule I followed was, I turned (a) into:

(a > 0) for comparing a counter/number.
(a != 0) for comparing an id, where there's no ordering.
(!!a) for pointers.

[commit-bot: I haz the power, 2013-03-29 01:42:36]

Failed to trigger a try job on linux_rel
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-03-29 01:42:47]

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

[commit-bot: I haz the power, 2013-03-29 03:01:17]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-03-29 04:35:47]

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

[commit-bot: I haz the power, 2013-03-29 06:26:10]

Failed to apply patch for cc/layers/heads_up_display_layer_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/layers/heads_up_display_layer_impl.cc
  Hunk #2 FAILED at 101.
  1 out of 2 hunks FAILED -- saving rejects to file
cc/layers/heads_up_display_layer_impl.cc.rej

Patch:       cc/layers/heads_up_display_layer_impl.cc
Index: cc/layers/heads_up_display_layer_impl.cc
diff --git a/cc/layers/heads_up_display_layer_impl.cc
b/cc/layers/heads_up_display_layer_impl.cc
index
270e04d772ab6d22ac031d996b30f48b2ff2ed24..3a8ddf35d52a315b4f593eb40dda1ad9bbc01e8d
100644
--- a/cc/layers/heads_up_display_layer_impl.cc
+++ b/cc/layers/heads_up_display_layer_impl.cc
@@ -69,10 +69,10 @@ double HeadsUpDisplayLayerImpl::Graph::UpdateUpperBound() {
 HeadsUpDisplayLayerImpl::HeadsUpDisplayLayerImpl(LayerTreeImpl* tree_impl,
                                                  int id)
     : LayerImpl(tree_impl, id),
-      fps_graph_(60.0, 80.0),
-      paint_time_graph_(16.0, 48.0),
       typeface_(skia::AdoptRef(
-          SkTypeface::CreateFromName("monospace", SkTypeface::kBold))) {}
+          SkTypeface::CreateFromName("monospace", SkTypeface::kBold))),
+      fps_graph_(60.0, 80.0),
+      paint_time_graph_(16.0, 48.0) {}
 
 HeadsUpDisplayLayerImpl::~HeadsUpDisplayLayerImpl() {}
 
@@ -101,7 +101,7 @@ void HeadsUpDisplayLayerImpl::WillDraw(ResourceProvider*
resource_provider) {
         bounds(), GL_RGBA, ResourceProvider::TextureUsageAny);
     // TODO(epenner): This texture was being used before SetPixels was called,
     // which is now not allowed (it's an uninitialized read). This should be
-    // fixed and this allocateForTesting() removed.
+    // fixed and this AllocateForTesting() removed.
     // http://crbug.com/166784
     resource_provider->AllocateForTesting(hud_texture_->id());
   }

[danakj, 2013-03-29 16:26:00]

Committed patchset #4 manually as r191364 (presubmit successful).

[tfarina, 2013-03-29 17:24:02]

Looks like Dana is TOO FAST for me!

I was working on this. :/

crbug.com/196342

[danakj, 2013-03-29 17:31:34]

On Fri, Mar 29, 2013 at 1:24 PM, <tfarina@chromium.org> wrote:

> Looks like Dana is TOO FAST for me!
>
> I was working on this. :/
>
> crbug.com/196342


Yeh, sorry about that, I did forget entirely about that bug. I was trying
hard to get all the warnings on ASAP so we can stop regressing style stuff.


>
>
>
https://codereview.chromium.**org/13206004/<https://codereview.chromium.org/1...
>

[danakj, 2013-03-29 18:19:15]

Committed patchset #5 manually as r191400 (presubmit successful).

[danakj, 2013-03-29 19:06:10]

ChromeOS builds with completely different set of warnings, and this has been
reverted twice now. I'm going to land this without the flag-flip so other
patches don't get constant churn.

Then I'll do the flag-flip plus a few more fixes for cros in another CL>

[danakj, 2013-03-29 19:17:42]

Committed patchset #6 manually as r191417 (presubmit successful).