Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1084)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 11624021: Move tracing component to src/components (Closed)

Created:
8 years ago by kaiwang
Modified:
7 years, 1 month ago
Reviewers:
nduca, Jói, jam, Nico, yzshen1, jbauman
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Namespace change #

Total comments: 2

Patch Set 3 : Patch by Joi #

Patch Set 4 : Merge LKGR #

Total comments: 6

Patch Set 5 : Address comments #

Patch Set 6 : Add a DEPS rule back #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -375 lines) Patch
M components/OWNERS View 1 2 1 chunk +3 lines, -0 lines 0 comments Download
A + components/components_tracing.gyp View 1 chunk +4 lines, -4 lines 1 comment Download
A + components/components_tracing_untrusted.gyp View 1 chunk +4 lines, -4 lines 0 comments Download
A + components/tracing/DEPS View 1 2 3 4 5 1 chunk +1 line, -2 lines 0 comments Download
A + components/tracing/OWNERS View 0 chunks +-1 lines, --1 lines 0 comments Download
A + components/tracing/child_trace_message_filter.h View 1 3 chunks +5 lines, -5 lines 0 comments Download
A + components/tracing/child_trace_message_filter.cc View 1 2 chunks +4 lines, -4 lines 0 comments Download
A + components/tracing/tracing_messages.h View 0 chunks +-1 lines, --1 lines 0 comments Download
A + components/tracing/tracing_messages.cc View 1 chunk +6 lines, -6 lines 0 comments Download
M content/browser/DEPS View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/trace_message_filter.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/common/DEPS View 1 chunk +1 line, -1 line 0 comments Download
M content/common/child_thread.cc View 1 3 chunks +2 lines, -2 lines 0 comments Download
D content/components/tracing/DEPS View 1 chunk +0 lines, -9 lines 0 comments Download
D content/components/tracing/OWNERS View 1 chunk +0 lines, -2 lines 0 comments Download
D content/components/tracing/child_trace_message_filter.h View 1 chunk +0 lines, -53 lines 0 comments Download
D content/components/tracing/child_trace_message_filter.cc View 1 chunk +0 lines, -117 lines 0 comments Download
D content/components/tracing/tracing_messages.h View 1 chunk +0 lines, -56 lines 0 comments Download
D content/components/tracing/tracing_messages.cc View 1 chunk +0 lines, -34 lines 0 comments Download
M content/content_common.gypi View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
D content/content_components_tracing.gyp View 1 chunk +0 lines, -27 lines 0 comments Download
D content/content_components_tracing_untrusted.gyp View 1 chunk +0 lines, -41 lines 0 comments Download
M ppapi/native_client/native_client.gyp View 1 chunk +1 line, -1 line 0 comments Download
M ppapi/ppapi_ipc_proxy_untrusted.gyp View 1 chunk +1 line, -1 line 0 comments Download
M ppapi/ppapi_ipc_untrusted.gyp View 1 chunk +1 line, -1 line 0 comments Download
M ppapi/proxy/DEPS View 1 chunk +2 lines, -2 lines 0 comments Download
M ppapi/proxy/plugin_main_nacl.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 22 (0 generated)
kaiwang
8 years ago (2012-12-21 05:12:27 UTC) #1
Jói
Looks good, see one nit below. Let's get approval from one of the two OWNERS ...
8 years ago (2012-12-21 11:09:44 UTC) #2
Jói
+nduca, jbauman (whoever gets to this first) for LGTM since they are owners of this ...
8 years ago (2012-12-21 11:13:45 UTC) #3
nduca
lgtm if you need it. I dont really get why this tiny part of tracing ...
8 years ago (2012-12-21 18:13:52 UTC) #4
jbauman
On 2012/12/21 18:13:52, nduca wrote: > lgtm if you need it. I dont really get ...
8 years ago (2012-12-21 18:16:03 UTC) #5
kaiwang
https://codereview.chromium.org/11624021/diff/4001/components/tracing/DEPS File components/tracing/DEPS (right): https://codereview.chromium.org/11624021/diff/4001/components/tracing/DEPS#newcode7 components/tracing/DEPS:7: # anything else in content. On 2012/12/21 11:09:44, Jói ...
8 years ago (2012-12-21 18:57:43 UTC) #6
nduca
Thanks John. Shows what I know. :)
8 years ago (2012-12-21 18:58:50 UTC) #7
jbauman
lgtm, didn't really make sense to have this under src/content anyway.
8 years ago (2012-12-21 21:47:46 UTC) #8
kaiwang
+ jam for content review + yzshen for ppapi
7 years, 12 months ago (2012-12-27 21:19:31 UTC) #9
yzshen1
On 2012/12/27 21:19:31, kaiwang wrote: > + jam for content review > + yzshen for ...
7 years, 12 months ago (2012-12-27 21:27:40 UTC) #10
jam
lgtm with nits https://codereview.chromium.org/11624021/diff/15002/components/tracing/DEPS File components/tracing/DEPS (right): https://codereview.chromium.org/11624021/diff/15002/components/tracing/DEPS#newcode4 components/tracing/DEPS:4: "+components/tracing", I don't think this is ...
7 years, 12 months ago (2012-12-27 21:44:25 UTC) #11
kaiwang
https://codereview.chromium.org/11624021/diff/15002/components/tracing/DEPS File components/tracing/DEPS (right): https://codereview.chromium.org/11624021/diff/15002/components/tracing/DEPS#newcode4 components/tracing/DEPS:4: "+components/tracing", On 2012/12/27 21:44:25, John Abd-El-Malek wrote: > I ...
7 years, 12 months ago (2012-12-27 22:08:31 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kaiwang@chromium.org/11624021/24001
7 years, 12 months ago (2012-12-27 22:09:53 UTC) #13
jam
https://chromiumcodereview.appspot.com/11624021/diff/15002/components/tracing/DEPS File components/tracing/DEPS (right): https://chromiumcodereview.appspot.com/11624021/diff/15002/components/tracing/DEPS#newcode8 components/tracing/DEPS:8: "-content", On 2012/12/27 22:08:31, kaiwang wrote: > On 2012/12/27 ...
7 years, 12 months ago (2012-12-27 22:44:15 UTC) #14
kaiwang
https://chromiumcodereview.appspot.com/11624021/diff/15002/components/tracing/DEPS File components/tracing/DEPS (right): https://chromiumcodereview.appspot.com/11624021/diff/15002/components/tracing/DEPS#newcode8 components/tracing/DEPS:8: "-content", OK, will add it back
7 years, 12 months ago (2012-12-27 22:57:26 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kaiwang@chromium.org/11624021/30001
7 years, 12 months ago (2012-12-27 22:57:57 UTC) #16
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 12 months ago (2012-12-27 23:37:45 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kaiwang@chromium.org/11624021/30001
7 years, 12 months ago (2012-12-28 00:13:31 UTC) #18
commit-bot: I haz the power
Change committed as 174716
7 years, 12 months ago (2012-12-28 01:14:28 UTC) #19
Nico
https://chromiumcodereview.appspot.com/11624021/diff/30001/components/components_tracing.gyp File components/components_tracing.gyp (right): https://chromiumcodereview.appspot.com/11624021/diff/30001/components/components_tracing.gyp#newcode11 components/components_tracing.gyp:11: 'defines!': ['CONTENT_IMPLEMENTATION'], This needs + 'variables': { + 'chromium_code': ...
7 years, 2 months ago (2013-10-23 19:00:03 UTC) #20
Nico
On 2013/10/23 19:00:03, Nico (in Tokyo until Nov 25) wrote: > https://chromiumcodereview.appspot.com/11624021/diff/30001/components/components_tracing.gyp > File components/components_tracing.gyp ...
7 years, 1 month ago (2013-11-17 09:40:36 UTC) #21
Jói
7 years, 1 month ago (2013-11-18 13:26:06 UTC) #22
Message was sent while issue was closed. 
On 2013/11/17 09:40:36, Nico (in Tokyo until Nov 25) wrote:
> On 2013/10/23 19:00:03, Nico (in Tokyo until Nov 25) wrote:
> >
>
https://chromiumcodereview.appspot.com/11624021/diff/30001/components/compone...
> > File components/components_tracing.gyp (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/11624021/diff/30001/components/compone...
> > components/components_tracing.gyp:11: 'defines!':
['CONTENT_IMPLEMENTATION'],
> > This needs
> > 
> > +  'variables': {
> > +    'chromium_code': 1,
> > +  },
> > 
> > at the top, else files in this target are built without warnings.
> > 
> > Also, do you still need the defines! line?
> 
> joi, kaiwan: ping ^

Sorry about that, I missed your question 3 weeks ago. See
https://codereview.chromium.org/62833009/

Powered by Google App Engine
This is Rietveld 408576698