Adding a mojo interface to the mojo spy.

Adding more guts to the mojo spy.

This CL fleshes out the basic architecture of the spy. Introducing SpyServerImpl and enough code to operate the "start" and "stop" commands of the front-end.

Still a quite a few loose ends, specially on teardown.

I updated the design doc to reflect this CL:
https://docs.google.com/a/chromium.org/document/d/11FKYXf9mSohlsgl4JmGlyWE1ScX3DKdssdjub63tkwA/edit?usp=sharing


BUG=360188
TEST=manual via test/spy_repl_test.html

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

[cpu_(ooo_6.6-7.5), 2014-05-17 01:08:57]

the code is pretty prosaic save the part where I use a mojo messagepipe instead
of PostTask. See the design doc last two pages to get what I mean.

[darin (slow to review), 2014-05-17 02:37:57]

LGTM

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/public/spy.mojom
File mojo/spy/public/spy.mojom (right):

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/public/spy.mojo...
mojo/spy/public/spy.mojom:44: FatalError(Result result);
nit: should this be OnFatalError?

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy.cc
File mojo/spy/spy.cc (right):

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy.cc#newcode216
mojo/spy/spy.cc:216: spy_server_ = new SpyServerImpl();
Does SpyServerImpl really need to be ref-counted? Do you plan to use spy_server_
from the Spy class (outside of this constructor)? Or can its ownership be passed
on to the SpyInterceptor? If so, then it could be stored in a scoped_ptr, right?

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl.cc
File mojo/spy/spy_server_impl.cc (right):

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl...
mojo/spy/spy_server_impl.cc:73: void SpyServerImpl::OnIntercept(GURL url) {
nit: |const GURL&|

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl...
mojo/spy/spy_server_impl.cc:80: client()->FatalError(spy_api::NO_MORE_IDS);
is this something you plan to mitigate later or do you expect this will not come
up in practice?

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl.h
File mojo/spy/spy_server_impl.h (right):

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl...
mojo/spy/spy_server_impl.h:46: struct Item;
nit: does this need to be a public struct?

[cpu_(ooo_6.6-7.5), 2014-05-19 20:09:18]

The CQ bit was checked by cpu@chromium.org

[cpu_(ooo_6.6-7.5), 2014-05-19 20:09:34]

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/public/spy.mojom
File mojo/spy/public/spy.mojom (right):

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/public/spy.mojo...
mojo/spy/public/spy.mojom:44: FatalError(Result result);
On 2014/05/17 02:37:58, darin wrote:
> nit: should this be OnFatalError?

Done.

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy.cc
File mojo/spy/spy.cc (right):

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy.cc#newcode216
mojo/spy/spy.cc:216: spy_server_ = new SpyServerImpl();
On 2014/05/17 02:37:58, darin wrote:
> Does SpyServerImpl really need to be ref-counted? Do you plan to use
spy_server_
> from the Spy class (outside of this constructor)? Or can its ownership be
passed
> on to the SpyInterceptor? If so, then it could be stored in a scoped_ptr,
right?

Possibly. Right now the interceptor is the only name in town but it could be
that we can do things without it. Tracing maybe.

The refcounting is really needed for PostTask'ing. I could unretained() but
seems worse.

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl.cc
File mojo/spy/spy_server_impl.cc (right):

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl...
mojo/spy/spy_server_impl.cc:73: void SpyServerImpl::OnIntercept(GURL url) {
On 2014/05/17 02:37:58, darin wrote:
> nit: |const GURL&|

Done.

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl...
mojo/spy/spy_server_impl.cc:80: client()->FatalError(spy_api::NO_MORE_IDS);
On 2014/05/17 02:37:58, darin wrote:
> is this something you plan to mitigate later or do you expect this will not
come
> up in practice?

It is an interesting question. In theory the frontend could make me hold to a
copied message forever, so I expect I will run out of ram before I loop around
on ids.

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl.h
File mojo/spy/spy_server_impl.h (right):

https://codereview.chromium.org/284743002/diff/70001/mojo/spy/spy_server_impl...
mojo/spy/spy_server_impl.h:46: struct Item;
On 2014/05/17 02:37:58, darin wrote:
> nit: does this need to be a public struct?

Done.

[commit-bot: I haz the power, 2014-05-19 20:09:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cpu@chromium.org/284743002/90001

[commit-bot: I haz the power, 2014-05-20 00:29:53]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-20 00:57:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-20 00:58:01]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)

[cpu_(ooo_6.6-7.5), 2014-05-22 21:36:52]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2014-05-22 21:37:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cpu@chromium.org/284743002/110001

[cpu_(ooo_6.6-7.5), 2014-05-22 23:18:41]

The CQ bit was unchecked by cpu@chromium.org

[cpu_(ooo_6.6-7.5), 2014-05-22 23:22:24]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2014-05-22 23:23:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cpu@chromium.org/284743002/130001

[commit-bot: I haz the power, 2014-05-23 09:50:19]

Change committed as 272458