Add the tracing manager

Add the tracing manager

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

[Zachary Kuznia, 2013-07-19 00:40:25]

Please take a look.

[rkc, 2013-07-22 22:59:02]

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
File chrome/browser/feedback/tracing_manager.h (right):

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
chrome/browser/feedback/tracing_manager.h:18: typedef base::Callback<void(const
std::string& trace_data)> TraceDataCallback;
Since the trace data can be very large, it may make sense to pass a scoper to
the string data instead of the string itself.

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
chrome/browser/feedback/tracing_manager.h:20: class TracingManager : public
content::TraceSubscriber {
Add class comment.

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
chrome/browser/feedback/tracing_manager.h:25: static scoped_ptr<TracingManager>
Create();
Using scoped ptrs for singleton classes doesn't work very well. Is there a
reason why this class needs to be singleton? If not, we should keep the Create()
to ensure this class is created scoped, but get rid of the Get(). If not, we
should just implement this as a Singleton with a unified Create/Get method.

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
chrome/browser/feedback/tracing_manager.h:58: std::map<int, std::string>
trace_data_;
Same reasoning as the callback def above, should store a scoper to a string
instead.

[Zachary Kuznia, 2013-07-23 01:53:24]

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
File chrome/browser/feedback/tracing_manager.h (right):

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
chrome/browser/feedback/tracing_manager.h:18: typedef base::Callback<void(const
std::string& trace_data)> TraceDataCallback;
On 2013/07/22 22:59:02, Rahul Chaturvedi wrote:
> Since the trace data can be very large, it may make sense to pass a scoper to
> the string data instead of the string itself.

Done.

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
chrome/browser/feedback/tracing_manager.h:20: class TracingManager : public
content::TraceSubscriber {
On 2013/07/22 22:59:02, Rahul Chaturvedi wrote:
> Add class comment.
> 

Done.

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
chrome/browser/feedback/tracing_manager.h:25: static scoped_ptr<TracingManager>
Create();
On 2013/07/22 22:59:02, Rahul Chaturvedi wrote:
> Using scoped ptrs for singleton classes doesn't work very well. Is there a
> reason why this class needs to be singleton? If not, we should keep the
Create()
> to ensure this class is created scoped, but get rid of the Get(). If not, we
> should just implement this as a Singleton with a unified Create/Get method.
> 
> 
> 

I tried to explain this in the class comment as we discussed offline.

https://codereview.chromium.org/19762005/diff/1/chrome/browser/feedback/traci...
chrome/browser/feedback/tracing_manager.h:58: std::map<int, std::string>
trace_data_;
On 2013/07/22 22:59:02, Rahul Chaturvedi wrote:
> Same reasoning as the callback def above, should store a scoper to a string
> instead.

Done.

[rkc, 2013-07-23 21:24:51]

lgtm

[commit-bot: I haz the power, 2013-07-23 21:56:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zork@chromium.org/19762005/5001

[commit-bot: I haz the power, 2013-07-23 22:29:18]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
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.

[rkc, 2013-07-23 22:42:25]

still lgtm

[commit-bot: I haz the power, 2013-07-24 00:20:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zork@chromium.org/19762005/46001

[commit-bot: I haz the power, 2013-07-24 00:31:53]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-07-24 00:42:23]

Change committed as 213299