Add a mechanism to let TracingController invoke JavaScript via TracingUI

Add a mechanism to let TracingController invoke JavaScript via TracingUI

This CL enables TracingController to call JavaScript in about:tracing tabs
when a monitoring state is changed. If one tab changes the monitoring state, TracingController notifies
the change to all other about:tracing tabs.
That way we can keep the monitoring state displayed in all about:tracing tabs consistent.

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

[haraken, 2014-01-08 07:06:49]

PTAL

[dsinclair, 2014-01-08 15:57:26]

https://codereview.chromium.org/127703002/diff/60001/content/browser/tracing/...
File content/browser/tracing/tracing_controller_impl.cc (right):

https://codereview.chromium.org/127703002/diff/60001/content/browser/tracing/...
content/browser/tracing/tracing_controller_impl.cc:811: {
Should we add a:

if (is_monitoring == is_monitoring_)
  return;

?

https://codereview.chromium.org/127703002/diff/60001/content/browser/tracing/...
File content/browser/tracing/tracing_ui.cc (right):

https://codereview.chromium.org/127703002/diff/60001/content/browser/tracing/...
content/browser/tracing/tracing_ui.cc:250: {
nit: move to line above.

https://codereview.chromium.org/127703002/diff/60001/content/browser/tracing/...
content/browser/tracing/tracing_ui.cc:255: {
nit: ditto

https://codereview.chromium.org/127703002/diff/60001/content/public/browser/t...
File content/public/browser/tracing_controller.h (right):

https://codereview.chromium.org/127703002/diff/60001/content/public/browser/t...
content/public/browser/tracing_controller.h:165: // Regsiter/Unregister tracing
UI that is using this TracingController.
nit: s/Regsiter/Register

[nduca, 2014-01-08 18:12:27]

lgtm awesome but wait for dans review as well

[dsinclair, 2014-01-08 19:12:36]

I was just nits, so lgtm once those are fixed so you aren't blocked overnight.

[haraken, 2014-01-09 00:48:18]

Thanks, all comments addressed.

jam: Would you take a look at the content/public/ change?

[jam, 2014-01-09 18:11:45]

https://codereview.chromium.org/127703002/diff/130001/content/public/browser/...
File content/public/browser/tracing_controller.h (right):

https://codereview.chromium.org/127703002/diff/130001/content/public/browser/...
content/public/browser/tracing_controller.h:166: virtual void
RegisterTracingUI(TracingUI* tracing_ui) = 0;
since these are methods called internally from content, they shold go on
TracingControllerImpl instead of the public interface. see
http://www.chromium.org/developers/content-module/content-api

[haraken, 2014-01-10 01:02:37]

https://codereview.chromium.org/127703002/diff/130001/content/public/browser/...
File content/public/browser/tracing_controller.h (right):

https://codereview.chromium.org/127703002/diff/130001/content/public/browser/...
content/public/browser/tracing_controller.h:166: virtual void
RegisterTracingUI(TracingUI* tracing_ui) = 0;
On 2014/01/09 18:11:45, jam wrote:
> since these are methods called internally from content, they shold go on
> TracingControllerImpl instead of the public interface. see
> http://www.chromium.org/developers/content-module/content-api

Thanks, removed from public APIs.

[commit-bot: I haz the power, 2014-01-10 01:09:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/127703002/210001

[commit-bot: I haz the power, 2014-01-10 01:46:38]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[haraken, 2014-01-10 13:45:57]

hmm, android bots are failing with the following compile error:

> in function
content::TracingControllerImpl::OnMonitoringStateChanged(bool):tracing_controller_impl.cc
> error: undefined reference to
'content::TracingUI::OnMonitoringStateChanged(bool)'
> collect2: ld returned 1 exit status

I'm not sure why the link fails. Here TracingControllerImpl is just accessing
TracingUI's method. Both TracingControllerImpl and TracingUI are in content/...

Do you have any idea?

[Xianzhu, 2014-01-10 17:16:26]

On 2014/01/10 13:45:57, haraken wrote:
> hmm, android bots are failing with the following compile error:
> 
> > in function
>
content::TracingControllerImpl::OnMonitoringStateChanged(bool):tracing_controller_impl.cc
> > error: undefined reference to
> 'content::TracingUI::OnMonitoringStateChanged(bool)'
> > collect2: ld returned 1 exit status
> 
> I'm not sure why the link fails. Here TracingControllerImpl is just accessing
> TracingUI's method. Both TracingControllerImpl and TracingUI are in
content/...
> 
> Do you have any idea?

tracing_ui.cc is excluded on Android. You can try to remove
^browser/tracing/tracing_ui from the exclude list in content_browser_gypi.

[jam, 2014-01-10 20:24:13]

On 2014/01/10 17:16:26, Xianzhu wrote:
> On 2014/01/10 13:45:57, haraken wrote:
> > hmm, android bots are failing with the following compile error:
> > 
> > > in function
> >
>
content::TracingControllerImpl::OnMonitoringStateChanged(bool):tracing_controller_impl.cc
> > > error: undefined reference to
> > 'content::TracingUI::OnMonitoringStateChanged(bool)'
> > > collect2: ld returned 1 exit status
> > 
> > I'm not sure why the link fails. Here TracingControllerImpl is just
accessing
> > TracingUI's method. Both TracingControllerImpl and TracingUI are in
> content/...
> > 
> > Do you have any idea?
> 
> tracing_ui.cc is excluded on Android. You can try to remove
> ^browser/tracing/tracing_ui from the exclude list in content_browser_gypi.

no need to include that code on android since it's not used there. so just don't
call that code on android

[jam, 2014-01-10 20:24:58]

https://codereview.chromium.org/127703002/diff/450001/content/public/browser/...
File content/public/browser/tracing_controller.h (right):

https://codereview.chromium.org/127703002/diff/450001/content/public/browser/...
content/public/browser/tracing_controller.h:21: class TracingUI;
remove

[commit-bot: I haz the power, 2014-01-14 06:16:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/127703002/610001

[commit-bot: I haz the power, 2014-01-14 08:35:08]

Retried try job too often on win_rel for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-14 09:29:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/127703002/610001

[commit-bot: I haz the power, 2014-01-14 14:56:02]

Change committed as 244709