Add ApiActivityMonitor to decouple ActivityLog from low-level extensions code

Add ApiActivityMonitor to decouple ActivityLog from low-level extensions code

* Introduce ApiActivityMonitor interface and make ActivityLog implement it
* Use it to replace EventRouter's one-off EventDispatchObserver, which was
  kind of ugly.
* Use it for function call notifications from ExtensionFunctionDispatcher

This allows app_shell to use ExtensionFunctionDispatcher without an
ActivityLog and gets us another step closer to moving EFD into src/extensions.

BUG=332986, 339595
TEST=browser_tests Extension*

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

[James Cook, 2014-02-04 23:43:21]

yoz, PTAL

felt, FYI

[James Cook, 2014-02-05 23:28:10]

Friendly ping?

[Yoyo Zhou, 2014-02-06 00:31:02]

LGTM

https://chromiumcodereview.appspot.com/155183002/diff/100001/chrome/browser/e...
File chrome/browser/extensions/extension_function_dispatcher.cc (right):

https://chromiumcodereview.appspot.com/155183002/diff/100001/chrome/browser/e...
chrome/browser/extensions/extension_function_dispatcher.cc:71:
extensions::ExtensionsBrowserClient::Get()->GetApiActivityMonitor(
Since this won't change for the lifetime of EFD (which should be shorter than
that of ActivityLog), perhaps we should cache the monitor pointer. I'm not sure
if this is worth it though.

[James Cook, 2014-02-06 00:49:23]

https://codereview.chromium.org/155183002/diff/100001/chrome/browser/extensio...
File chrome/browser/extensions/extension_function_dispatcher.cc (right):

https://codereview.chromium.org/155183002/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/extension_function_dispatcher.cc:71:
extensions::ExtensionsBrowserClient::Get()->GetApiActivityMonitor(
On 2014/02/06 00:31:03, Yoyo Zhou wrote:
> Since this won't change for the lifetime of EFD (which should be shorter than
> that of ActivityLog), perhaps we should cache the monitor pointer. I'm not
sure
> if this is worth it though.

I'm not sure it's worth it either. One of the callers of this function is a
static method that doesn't have access to an ExtensionFunctionDispatcher object
and runs on the IO thread. I'd have to do some kind of lookup to find the EFD
and might have to jump between threads to get at a cached ApiActivityMonitor
pointer. I think it's simpler to just look it up each time.

If you feel strongly about it ping me and I can look at doing it in another CL.

[Yoyo Zhou, 2014-02-06 00:50:22]

On 2014/02/06 00:49:23, James Cook wrote:
>
https://codereview.chromium.org/155183002/diff/100001/chrome/browser/extensio...
> File chrome/browser/extensions/extension_function_dispatcher.cc (right):
> 
>
https://codereview.chromium.org/155183002/diff/100001/chrome/browser/extensio...
> chrome/browser/extensions/extension_function_dispatcher.cc:71:
> extensions::ExtensionsBrowserClient::Get()->GetApiActivityMonitor(
> On 2014/02/06 00:31:03, Yoyo Zhou wrote:
> > Since this won't change for the lifetime of EFD (which should be shorter
than
> > that of ActivityLog), perhaps we should cache the monitor pointer. I'm not
> sure
> > if this is worth it though.
> 
> I'm not sure it's worth it either. One of the callers of this function is a
> static method that doesn't have access to an ExtensionFunctionDispatcher
object
> and runs on the IO thread. I'd have to do some kind of lookup to find the EFD
> and might have to jump between threads to get at a cached ApiActivityMonitor
> pointer. I think it's simpler to just look it up each time.
> 
> If you feel strongly about it ping me and I can look at doing it in another
CL.

Nah, this is fine.

[James Cook, 2014-02-06 00:55:42]

The CQ bit was checked by jamescook@chromium.org

[commit-bot: I haz the power, 2014-02-06 01:03:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamescook@chromium.org/155183002/190001

[commit-bot: I haz the power, 2014-02-06 10:03:45]

Change committed as 249324