New issue
Advanced search Search tips

Issue 732564 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Pass EventFilteringInfo directly over IPC

Project Member Reported by rdevlin....@chromium.org, Jun 12 2017

Issue description

Right now, we serialize EventFilteringInfo into a dictionary to pass it over IPC. [1]  Instead, we should just add serialization rules and avoid the JSON-ification.

This is also a good time to simplify EventFilteringInfo.

[1] https://chromium.googlesource.com/chromium/src/+/4396b7a7ed40e6260568f14f52649d998a121b8d/extensions/browser/event_router.cc#94
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb

commit 22da9e8bfbd02c8052f076e59e203bd2dd74c4eb
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Wed Jun 14 16:28:37 2017

[Extensions] Simplify EventFilteringInfo

EventFilteringInfo currently has a bunch of parameters and boolean flags
indicating whether or not it "has" those parameters. This can be greatly
simplified with base::Optional. Use base::Optional for all the members
in EventFilteringInfo and, since it only exposed getters/setters, make
it a struct.

This cleanup will be very useful in passing the filtering info directly
over IPC.

BUG= 732564 

Review-Url: https://codereview.chromium.org/2937623002
Cr-Commit-Position: refs/heads/master@{#479412}

[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/chrome/browser/extensions/api/mdns/mdns_api.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/chrome/browser/extensions/api/tabs/windows_event_router.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/chrome/browser/extensions/api/web_navigation/web_navigation_api_helpers.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/chrome/browser/extensions/menu_manager.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/browser/event_listener_map_unittest.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/browser/guest_view/extensions_guest_view_manager_delegate.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/common/event_filter.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/common/event_filter_unittest.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/common/event_filtering_info.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/common/event_filtering_info.h
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/common/event_matcher.cc
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/common/event_matcher.h
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/renderer/api_event_handler.h
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/renderer/api_event_listeners.h
[modify] https://crrev.com/22da9e8bfbd02c8052f076e59e203bd2dd74c4eb/extensions/renderer/event_emitter.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32f6a61d04a6d71f66853223f4e864ec424455c3

commit 32f6a61d04a6d71f66853223f4e864ec424455c3
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Thu Jun 15 19:14:56 2017

[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}

[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/browser/event_router.cc
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/common/event_filtering_info.cc
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/common/event_filtering_info.h
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/common/extension_messages.h
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/api_event_listeners_unittest.cc
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/dispatcher.cc
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/dispatcher.h
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/event_bindings.cc
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/event_bindings.h
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/extension_bindings_system.h
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/js_extension_bindings_system.cc
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/js_extension_bindings_system.h
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/native_extension_bindings_system.cc
[modify] https://crrev.com/32f6a61d04a6d71f66853223f4e864ec424455c3/extensions/renderer/native_extension_bindings_system.h

Status: Fixed (was: Started)

Sign in to add a comment