[Extensions] Add renderer-side logic to short circuit activity logging

[Extensions] Add renderer-side logic to short circuit activity logging

Revision a84983c58d89dbb83fc97eb24ad70f5780375dd9 added logic to short-
circuit activity logging on the browser side so that we don't log
anything if the user doesn't have an activity-logging extension enabled.
This patch adds logic to the renderer side to avoid adding the
DOMActivityLoggers and APIActivityLoggers that would generate additional
IPCs sent to the browser, in addition to requiring each DOM modification
in the given world to be logged. Now, each of these is only logged in
the case of the user having an active activity-logging extension.

BUG=620359
Committed: https://crrev.com/6fba7ec7467f6f6b3a48636a6006425b5f748f5a
Cr-Commit-Position: refs/heads/master@{#401868}

[Devlin, 2016-06-22 21:27:45]

rdevlin.cronin@chromium.org changed reviewers:
+ asargent@chromium.org, felt@chromium.org

[Devlin, 2016-06-22 21:27:45]

Renderer side!

[asargent_no_longer_on_chrome, 2016-06-23 13:35:56]

https://codereview.chromium.org/2089333002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/activity_log/activity_log.h (right):

https://codereview.chromium.org/2089333002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/activity_log/activity_log.h:121: bool is_active()
const { return is_active_; }
nit: this could use a comment with an explanation of what "active" means

https://codereview.chromium.org/2089333002/diff/1/extensions/renderer/dispatc...
File extensions/renderer/dispatcher.cc (right):

https://codereview.chromium.org/2089333002/diff/1/extensions/renderer/dispatc...
extensions/renderer/dispatcher.cc:1023: extension_id);
Does this mean that even after the first activity-logging extension is
installed, we still won't get logging of the activity for the lifetime of this
RenderView?

[Devlin, 2016-06-23 16:59:02]

https://codereview.chromium.org/2089333002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/activity_log/activity_log.h (right):

https://codereview.chromium.org/2089333002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/activity_log/activity_log.h:121: bool is_active()
const { return is_active_; }
On 2016/06/23 13:35:56, Antony Sargent wrote:
> nit: this could use a comment with an explanation of what "active" means

There is a comment on the |is_active_|, and I think for accessors we don't add
additional documentation (because one or the other will likely forget to be
updated).  But I added some more detail to the |is_active_| description, since
it was rather sparse.

https://codereview.chromium.org/2089333002/diff/1/extensions/renderer/dispatc...
File extensions/renderer/dispatcher.cc (right):

https://codereview.chromium.org/2089333002/diff/1/extensions/renderer/dispatc...
extensions/renderer/dispatcher.cc:1023: extension_id);
On 2016/06/23 13:35:56, Antony Sargent wrote:
> Does this mean that even after the first activity-logging extension is
> installed, we still won't get logging of the activity for the lifetime of this
> RenderView?

Good point!  Added this into OnSetActivityLoggingEnabled() so that it works
again.

[asargent_no_longer_on_chrome, 2016-06-23 21:07:04]

lgtm w/ one question

https://codereview.chromium.org/2089333002/diff/20001/extensions/renderer/dis...
File extensions/renderer/dispatcher.cc (right):

https://codereview.chromium.org/2089333002/diff/20001/extensions/renderer/dis...
extensions/renderer/dispatcher.cc:1266:
DOMActivityLogger::AttachToWorld(DOMActivityLogger::kMainWorldId, id);
Shouldn't you only do this if |enabled| is true?

[Devlin, 2016-06-24 01:13:53]

https://codereview.chromium.org/2089333002/diff/20001/extensions/renderer/dis...
File extensions/renderer/dispatcher.cc (right):

https://codereview.chromium.org/2089333002/diff/20001/extensions/renderer/dis...
extensions/renderer/dispatcher.cc:1266:
DOMActivityLogger::AttachToWorld(DOMActivityLogger::kMainWorldId, id);
On 2016/06/23 21:07:03, Antony Sargent wrote:
> Shouldn't you only do this if |enabled| is true?

Absolutely.  Thanks for catching this.

[Devlin, 2016-06-24 01:13:57]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2016-06-24 01:13:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
asargent@chromium.org
Link to the patchset: https://codereview.chromium.org/2089333002/#ps40001
(title: "Guard adding dom activity loggers")

[commit-bot: I haz the power, 2016-06-24 01:14:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2089333002/40001

[commit-bot: I haz the power, 2016-06-24 01:24:40]

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

[commit-bot: I haz the power, 2016-06-24 01:24:41]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Devlin, 2016-06-24 01:26:14]

rdevlin.cronin@chromium.org changed reviewers:
+ dcheng@chromium.org

[Devlin, 2016-06-24 01:26:15]

+dcheng for extension_messages.h

[dcheng, 2016-06-24 08:00:52]

ipc lgtm

[Devlin, 2016-06-24 15:03:32]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2016-06-24 15:04:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-24 16:15:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-24 16:17:12]

Description was changed from

==========
[Extensions] Add renderer-side logic to short circuit activity logging

Revision a84983c58d89dbb83fc97eb24ad70f5780375dd9 added logic to short-
circuit activity logging on the browser side so that we don't log
anything if the user doesn't have an activity-logging extension enabled.
This patch adds logic to the renderer side to avoid adding the
DOMActivityLoggers and APIActivityLoggers that would generate additional
IPCs sent to the browser, in addition to requiring each DOM modification
in the given world to be logged. Now, each of these is only logged in
the case of the user having an active activity-logging extension.

BUG=620359
==========

to

==========
[Extensions] Add renderer-side logic to short circuit activity logging

Revision a84983c58d89dbb83fc97eb24ad70f5780375dd9 added logic to short-
circuit activity logging on the browser side so that we don't log
anything if the user doesn't have an activity-logging extension enabled.
This patch adds logic to the renderer side to avoid adding the
DOMActivityLoggers and APIActivityLoggers that would generate additional
IPCs sent to the browser, in addition to requiring each DOM modification
in the given world to be logged. Now, each of these is only logged in
the case of the user having an active activity-logging extension.

BUG=620359

Committed: https://crrev.com/6fba7ec7467f6f6b3a48636a6006425b5f748f5a
Cr-Commit-Position: refs/heads/master@{#401868}
==========

[commit-bot: I haz the power, 2016-06-24 16:17:14]

Patchset 3 (id:??) landed as
https://crrev.com/6fba7ec7467f6f6b3a48636a6006425b5f748f5a
Cr-Commit-Position: refs/heads/master@{#401868}