[Extensions] Pass EventFilteringInfo directly in DispatchEvent message

[Extensions] Pass EventFilteringInfo directly in DispatchEvent message

Instead of serializing the EventFilteringInfo as a base::Value and
passing it to the renderer, and then deserialzing it again, just
pass the struct directly. This should be more efficient and
perhaps more secure (since it is more easily validated in the
messages).

BUG=732564
Review-Url: https://codereview.chromium.org/2940893002
Cr-Commit-Position: refs/heads/master@{#479775}
Committed: https://chromium.googlesource.com/chromium/src/+/32f6a61d04a6d71f66853223f4e864ec424455c3

[Devlin, 2017-06-14 16:02:16]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 16:02:48]

Dry run: 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, 2017-06-14 16:05:46]

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

[commit-bot: I haz the power, 2017-06-14 16:05:47]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Devlin, 2017-06-14 19:32:37]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 19:33:09]

Dry run: 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, 2017-06-14 20:15:27]

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

[commit-bot: I haz the power, 2017-06-14 20:15:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Devlin, 2017-06-14 21:29:14]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-14 21:29:37]

Dry run: 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, 2017-06-14 22:41:54]

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

[commit-bot: I haz the power, 2017-06-14 22:41:55]

Dry run: This issue passed the CQ dry run.

[Devlin, 2017-06-15 01:19:28]

Patchset #1 (id:1) has been deleted

[Devlin, 2017-06-15 01:19:37]

Patchset #1 (id:20001) has been deleted

[Devlin, 2017-06-15 01:19:44]

Patchset #1 (id:40001) has been deleted

[Devlin, 2017-06-15 01:20:07]

Description was changed from

==========
pass directly
BUG=
==========

to

==========
[Extensions] Pass EventFilteringInfo directly in DispatchEvent message

Instead of serializing the EventFilteringInfo as a base::Value and
passing it to the renderer, and then deserialzing it again, just
pass the struct directly. This should be more efficient and
perhaps more secure (since it is more easily validated in the
messages).

BUG=732564
==========

[Devlin, 2017-06-15 01:20:24]

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

[Devlin, 2017-06-15 01:20:25]

lazyboy@, mind taking a look?

[Devlin, 2017-06-15 15:11:01]

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

[Devlin, 2017-06-15 15:11:02]

+Robert for extension_messages.h IPC review.

[lazyboy, 2017-06-15 17:12:23]

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/ext...
File extensions/renderer/extension_bindings_system.h (right):

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/ext...
extensions/renderer/extension_bindings_system.h:15: struct EventFilteringInfo;
nit: struct after all class declarations.

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/nat...
File extensions/renderer/native_extension_bindings_system.cc (right):

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/nat...
extensions/renderer/native_extension_bindings_system.cc:490: filtering_info ?
*filtering_info : EventFilteringInfo());
Is there a reason FireEventInContext doesn't take EventFilteringInfo* (instead
of const&)? We could avoid creating empty EventFilteringInfo() in that case, and
isn't that the common scenario?

[Robert Sesek, 2017-06-15 17:54:25]

LGTM

[Devlin, 2017-06-15 18:00:58]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[Devlin, 2017-06-15 18:01:07]

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/ext...
File extensions/renderer/extension_bindings_system.h (right):

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/ext...
extensions/renderer/extension_bindings_system.h:15: struct EventFilteringInfo;
On 2017/06/15 17:12:23, lazyboy wrote:
> nit: struct after all class declarations.

Done.

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/nat...
File extensions/renderer/native_extension_bindings_system.cc (right):

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/nat...
extensions/renderer/native_extension_bindings_system.cc:490: filtering_info ?
*filtering_info : EventFilteringInfo());
On 2017/06/15 17:12:23, lazyboy wrote:
> Is there a reason FireEventInContext doesn't take EventFilteringInfo* (instead
> of const&)? We could avoid creating empty EventFilteringInfo() in that case,
and
> isn't that the common scenario?

Nope, it probably should.  I noticed that too, and was going to make a change
separately (it's goes a bit far down).  Does that SGTY?  Or would you prefer it
in this CL?

[commit-bot: I haz the power, 2017-06-15 18:01:27]

Dry run: CQ is trying da patch.

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

[lazyboy, 2017-06-15 18:06:43]

lgtm

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/nat...
File extensions/renderer/native_extension_bindings_system.cc (right):

https://codereview.chromium.org/2940893002/diff/60001/extensions/renderer/nat...
extensions/renderer/native_extension_bindings_system.cc:490: filtering_info ?
*filtering_info : EventFilteringInfo());
On 2017/06/15 18:01:06, Devlin wrote:
> On 2017/06/15 17:12:23, lazyboy wrote:
> > Is there a reason FireEventInContext doesn't take EventFilteringInfo*
(instead
> > of const&)? We could avoid creating empty EventFilteringInfo() in that case,
> and
> > isn't that the common scenario?
> 
> Nope, it probably should.  I noticed that too, and was going to make a change
> separately (it's goes a bit far down).  Does that SGTY?  Or would you prefer
it
> in this CL?

Separate CL sounds good! Thanks.

[Devlin, 2017-06-15 18:13:51]

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

[Devlin, 2017-06-15 18:13:55]

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

[Devlin, 2017-06-15 18:13:55]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org
Link to the patchset: https://codereview.chromium.org/2940893002/#ps80001
(title: ".")

[commit-bot: I haz the power, 2017-06-15 18:14:36]

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, 2017-06-15 19:14:59]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1497550435151110,
"parent_rev": "835ecd1c6796e7c69f35a67668e2af7aa56a784b", "commit_rev":
"32f6a61d04a6d71f66853223f4e864ec424455c3"}

[commit-bot: I haz the power, 2017-06-15 19:15:13]

Description was changed from

==========
[Extensions] Pass EventFilteringInfo directly in DispatchEvent message

Instead of serializing the EventFilteringInfo as a base::Value and
passing it to the renderer, and then deserialzing it again, just
pass the struct directly. This should be more efficient and
perhaps more secure (since it is more easily validated in the
messages).

BUG=732564
==========

to

==========
[Extensions] Pass EventFilteringInfo directly in DispatchEvent message

Instead of serializing the EventFilteringInfo as a base::Value and
passing it to the renderer, and then deserialzing it again, just
pass the struct directly. This should be more efficient and
perhaps more secure (since it is more easily validated in the
messages).

BUG=732564

Review-Url: https://codereview.chromium.org/2940893002
Cr-Commit-Position: refs/heads/master@{#479775}
Committed:
https://chromium.googlesource.com/chromium/src/+/32f6a61d04a6d71f66853223f4e8...
==========

[commit-bot: I haz the power, 2017-06-15 19:15:15]

Committed patchset #2 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/32f6a61d04a6d71f66853223f4e8...