Make cc a component

Make cc a component

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

[piman, 2012-11-01 22:51:09]

This one looks like it's working.

[tfarina, 2012-11-01 22:54:22]

Dana, looks like Antonie bet me :(

[piman, 2012-11-01 22:54:39]

On 2012/11/01 22:51:09, piman wrote:
> This one looks like it's working.

Note: for this one I took a minimalist approach of only exporting what is needed
for Chrome and tests. It still ends up being a lot.

Most non-trivial changes are with template instantiations in
cc/occlusion_tracker.cc, cc/layer_iterator.cc and gyp stuff. The rest is just
adding CC_EXPORT where needed.

[tfarina, 2012-11-01 22:55:48]

Antonie, just checking, are you sure you have to export every single class from
cc? From my little understand you only need to export the ones called outside
the cc/ module.

[piman, 2012-11-01 22:58:00]

On Thu, Nov 1, 2012 at 3:55 PM, <tfarina@chromium.org> wrote:

> Antonie, just checking, are you sure you have to export every single class
> from
> cc? From my little understand you only need to export the ones called
> outside
> the cc/ module.
>

Tests are outside of the cc module, so they end up needing a bunch of
"internal" classes.
I tried hard to only export what was needed to get things to compile/link.


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

[danakj, 2012-11-01 22:58:24]

https://codereview.chromium.org/11365025/diff/12001/cc/occlusion_tracker.cc
File cc/occlusion_tracker.cc (right):

https://codereview.chromium.org/11365025/diff/12001/cc/occlusion_tracker.cc#n...
cc/occlusion_tracker.cc:466: // Declare the possible functions here for the
linker.
Comment is wrong now. This is much simpler.. did this move somewhere?

[piman, 2012-11-01 23:03:43]

On Thu, Nov 1, 2012 at 3:58 PM, <danakj@chromium.org> wrote:

>
> https://codereview.chromium.**org/11365025/diff/12001/cc/**
>
occlusion_tracker.cc<https://codereview.chromium.org/11365025/diff/12001/cc/occlusion_tracker.cc>
> File cc/occlusion_tracker.cc (right):
>
> https://codereview.chromium.**org/11365025/diff/12001/cc/**
>
occlusion_tracker.cc#**newcode466<https://codereview.chromium.org/11365025/diff/12001/cc/occlusion_tracker.cc#newcode466>
> cc/occlusion_tracker.cc:466: // Declare the possible functions here for
> the linker.
> Comment is wrong now. This is much simpler.. did this move somewhere?
>

I'll fix the comment. It didn't move anywhere, I'm instantiating the entire
class instead of a subset. MSVC has issues with some inline functions
otherwise (the sigh).


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

[jamesr, 2012-11-01 23:11:30]

I don't think you need webkit_wtf_support any more (as of ~ this weekend) - you
can probably try without that

lgtm

https://codereview.chromium.org/11365025/diff/12001/cc/video_layer_impl.h
File cc/video_layer_impl.h (right):

https://codereview.chromium.org/11365025/diff/12001/cc/video_layer_impl.h#new...
cc/video_layer_impl.h:32: , public WebKit::WebVideoFrameProvider::Client {
alignment here is wacky - the ',' was lined up with the ':'

[piman, 2012-11-02 00:13:37]

https://codereview.chromium.org/11365025/diff/12001/cc/occlusion_tracker.cc
File cc/occlusion_tracker.cc (right):

https://codereview.chromium.org/11365025/diff/12001/cc/occlusion_tracker.cc#n...
cc/occlusion_tracker.cc:466: // Declare the possible functions here for the
linker.
On 2012/11/01 22:58:24, danakj wrote:
> Comment is wrong now. This is much simpler.. did this move somewhere?

Done.

https://codereview.chromium.org/11365025/diff/12001/cc/video_layer_impl.h
File cc/video_layer_impl.h (right):

https://codereview.chromium.org/11365025/diff/12001/cc/video_layer_impl.h#new...
cc/video_layer_impl.h:32: , public WebKit::WebVideoFrameProvider::Client {
On 2012/11/01 23:11:30, jamesr wrote:
> alignment here is wacky - the ',' was lined up with the ':'

Done.

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

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/11365025/5004

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

Presubmit check for 11365025-5004 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome

Presubmit checks took 4.9s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[piman, 2012-11-02 00:17:19]

+ben for OWNERS in chrome/ (just a gyp file)

[danakj, 2012-11-02 00:53:05]

I just noticed other files do this in the header file:

#if !defined(COMPILER_MSVC)
extern template class PointBase<PointF, float, Vector2dF>;
#endif

Should we do this as well, instead of in the .cc, and without guards?

[piman, 2012-11-02 01:01:58]

On Thu, Nov 1, 2012 at 5:53 PM, <danakj@chromium.org> wrote:

> I just noticed other files do this in the header file:
>
> #if !defined(COMPILER_MSVC)
> extern template class PointBase<PointF, float, Vector2dF>;
> #endif
>
> Should we do this as well, instead of in the .cc, and without guards?
>

I haven't had to do this so far, and I'm afraid of touching it while it
passes on all the trybots :)


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

[Ben Goodger (Google), 2012-11-02 16:08:44]

lgtm feel free to tbr such gypness to me.

[commit-bot: I haz the power, 2012-11-02 16:17:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/11365025/4116

[commit-bot: I haz the power, 2012-11-02 16:44:40]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...
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, 2012-11-02 16:55:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/11365025/5008

[commit-bot: I haz the power, 2012-11-02 18:49:36]

Retried try job too often for step(s) content_browsertests

[commit-bot: I haz the power, 2012-11-02 18:51:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/11365025/5008

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

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-11-02 19:22:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/11365025/1211