EventRouter (e/b/event_router.cc) has been growing. Find out code and logic we can refactor. This is particularly worthwhile for extension service worker events. I'm going to assign to myself, but anyone should feel free to the cleanup.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/640dc5161179cd9058dd3c97788880a8ac4c7f4d commit 640dc5161179cd9058dd3c97788880a8ac4c7f4d Author: lazyboy <lazyboy@chromium.org> Date: Fri May 19 21:37:02 2017 Remove RuntimeAPIDelegate::GetPreviousVersion. We currently go to chrome/ to get the extension version, but we don't need to. ExtensionRegistry in extensions/ has the info already, move the function to ExtensionRegistry. Also, the name GetPreviousVersion is misleading. It only gives us the current version, so rename it to be simply GetVersion. BUG=723754 Test=None, internal only change. Review-Url: https://codereview.chromium.org/2892843002 Cr-Commit-Position: refs/heads/master@{#473332} [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/chrome/browser/extensions/api/runtime/chrome_runtime_api_delegate.cc [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/chrome/browser/extensions/api/runtime/chrome_runtime_api_delegate.h [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/extensions/browser/api/runtime/runtime_api.cc [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/extensions/browser/api/runtime/runtime_api_delegate.h [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/extensions/browser/extension_registry.cc [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/extensions/browser/extension_registry.h [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/extensions/browser/test_runtime_api_delegate.cc [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/extensions/browser/test_runtime_api_delegate.h [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/extensions/shell/browser/shell_runtime_api_delegate.cc [modify] https://crrev.com/640dc5161179cd9058dd3c97788880a8ac4c7f4d/extensions/shell/browser/shell_runtime_api_delegate.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59155a46351362462cecf6ed4b6c584f2519cc34 commit 59155a46351362462cecf6ed4b6c584f2519cc34 Author: lazyboy <lazyboy@chromium.org> Date: Wed May 24 22:23:35 2017 [Extensions] Make Event::restrict_to_browser_context const. In all of the places where clients were setting Event::restrict_to_browser_context, all of them were setting it right after creating an Event instance. There's already a version of constructor for that. Use it. TBR=dmazzoni@chromium.org for chrome/browser/speech/extension_api/* TBR=derat@chromium.org for chrome/browser/ui/ash/* BUG=723754 Test=None Review-Url: https://codereview.chromium.org/2898383002 Cr-Commit-Position: refs/heads/master@{#474448} [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/chromeos/extensions/dictionary_event_router.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/chromeos/extensions/ime_menu_event_router.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/chromeos/extensions/input_method_event_router.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/chromeos/file_manager/file_browser_handlers.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/activity_log_private/activity_log_private_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/automation_internal/automation_event_router.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/cookies/cookies_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/debugger/debugger_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/downloads/downloads_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/extension_action/extension_action_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/feedback_private/feedback_private_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/history/history_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/identity/web_auth_flow.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/input_ime/input_ime_api_chromeos.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/input_ime/input_ime_api_nonchromeos.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/mdns/mdns_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/omnibox/omnibox_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/preference/preference_helpers.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/signed_in_devices/signed_in_devices_manager.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/sync_file_system/extension_sync_event_observer.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/tabs/tabs_event_router.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/tabs/windows_event_router.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/api/web_navigation/web_navigation_api_helpers.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/event_router_forwarder.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/extension_keybinding_registry.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/extension_messages_apitest.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/menu_manager.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/extensions/permissions_updater.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/speech/extension_api/tts_engine_extension_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/speech/extension_api/tts_extension_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/chrome/browser/ui/ash/chrome_keyboard_ui.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/extensions/browser/api/app_runtime/app_runtime_api.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/extensions/browser/api/idle/idle_manager.cc [modify] https://crrev.com/59155a46351362462cecf6ed4b6c584f2519cc34/extensions/browser/event_router.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/75b9def1648c17c0edc926e7ea787519e9c59899 commit 75b9def1648c17c0edc926e7ea787519e9c59899 Author: lazyboy <lazyboy@chromium.org> Date: Tue Jun 06 18:56:59 2017 Remove NOTIFICATION_EXTENSION_ENABLED. Enablement can happen for the following 3 cases: 1. New extension is added 2. User enables a disabled extension 3. An extension is reloaded or a terminated extension is reloaded It seems we used this in two classes in a) theme_service.cc and b) event_router.cc a) For themes, this can be simplified to load the theme on OnExtensionLoaded. Themes don't have process and it can't be terminated. We still have to remember whether the load is due to a theme update or not (OnExtensionWillBeInstalled). It is possible to avoid this by remembering both current theme's id (already done) and version in the prefs. This CL does not do that. b) The primary reason of EventRouter code is to make sure we are aware of new lazy events on extension upgrade, and to dispatch onInstalled event. The CL wraps the logic to use OnExtensionWillBeInstalled + OnExtensionLoaded into a separate class: LazyEventDispatchUtil. However, there are also 2 other reasons for EventRouter code in b): b1) We also need to reconnect devtools to a reloaded event page. That was happening through event_router. Pull that logic into extension_service as extension_service is already responsible for remembering orphaned devtools. b2) A component extension reload does not go through ExtensionRegistry::TriggerOnInstalled, so it doesn't see any OnExtensionWillBeInstalled. Couple the logic with b1). TBR=benwells@chromium.org for added test in app_browsertest.cc BUG=723754, 411569 Test=No visible changes expected. Review-Url: https://codereview.chromium.org/2893693002 Cr-Commit-Position: refs/heads/master@{#477357} [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/browser/apps/app_browsertest.cc [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/browser/extensions/api/input_ime/input_ime_api.h [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/browser/extensions/api/signed_in_devices/signed_in_devices_manager.h [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/browser/extensions/events_apitest.cc [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/browser/extensions/extension_service.cc [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/browser/extensions/extension_service.h [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/browser/themes/theme_service.cc [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/new_event_in_new_version/pem.pem [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/new_event_in_new_version/v1/background.js [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/new_event_in_new_version/v1/manifest.json [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/new_event_in_new_version/v2/background.js [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/new_event_in_new_version/v2/manifest.json [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed/pem.pem [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed/v1/background.js [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed/v1/manifest.json [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed/v2/background.js [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed/v2/manifest.json [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed_permissions_increase/pem.pem [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed_permissions_increase/v1/background.js [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed_permissions_increase/v1/manifest.json [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed_permissions_increase/v2/background.js [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/api_test/lazy_events/on_installed_permissions_increase/v2/manifest.json [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/platform_apps/component_reload/background.js [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/chrome/test/data/extensions/platform_apps/component_reload/manifest.json [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/extensions/browser/BUILD.gn [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/extensions/browser/api/runtime/runtime_api.cc [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/extensions/browser/api/runtime/runtime_api.h [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/extensions/browser/event_router.cc [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/extensions/browser/event_router.h [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/extensions/browser/events/lazy_event_dispatch_util.cc [add] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/extensions/browser/events/lazy_event_dispatch_util.h [modify] https://crrev.com/75b9def1648c17c0edc926e7ea787519e9c59899/extensions/browser/notification_types.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e464732fb0179fa3514239419c804547a0f56b8d commit e464732fb0179fa3514239419c804547a0f56b8d Author: lazyboy <lazyboy@chromium.org> Date: Thu Jun 15 21:17:27 2017 Move lazy event dispatching code out of EventRouter. Separate out lazy background page logic from EventRouter to LazyEventDispatcher. This will facilitate introducing service worker (another "lazy context") lazy events to extensions. Also introduce LazyContextId to identify a lazy context. This is currently used only for identifying event page context, but in future will also identify extension service worker context. BUG=721147, 723754 Review-Url: https://codereview.chromium.org/2940883007 Cr-Commit-Position: refs/heads/master@{#479833} [modify] https://crrev.com/e464732fb0179fa3514239419c804547a0f56b8d/extensions/browser/BUILD.gn [modify] https://crrev.com/e464732fb0179fa3514239419c804547a0f56b8d/extensions/browser/event_router.cc [modify] https://crrev.com/e464732fb0179fa3514239419c804547a0f56b8d/extensions/browser/event_router.h [add] https://crrev.com/e464732fb0179fa3514239419c804547a0f56b8d/extensions/browser/events/lazy_event_dispatcher.cc [add] https://crrev.com/e464732fb0179fa3514239419c804547a0f56b8d/extensions/browser/events/lazy_event_dispatcher.h [add] https://crrev.com/e464732fb0179fa3514239419c804547a0f56b8d/extensions/browser/lazy_context_id.cc [add] https://crrev.com/e464732fb0179fa3514239419c804547a0f56b8d/extensions/browser/lazy_context_id.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a258b60661cbb815ffd100be05ec77d47066a74e commit a258b60661cbb815ffd100be05ec77d47066a74e Author: lazyboy <lazyboy@chromium.org> Date: Fri Jun 16 02:01:47 2017 Remove unused EventRouter member function definitions. Mistakenly left out from r479833. BUG=723754 Test=No visible changes expected. Review-Url: https://codereview.chromium.org/2943013002 Cr-Commit-Position: refs/heads/master@{#479917} [modify] https://crrev.com/a258b60661cbb815ffd100be05ec77d47066a74e/extensions/browser/event_router.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2f2cf6cddff6f31b6a3ba55655462a34fb32113f commit 2f2cf6cddff6f31b6a3ba55655462a34fb32113f Author: Michael Giuffrida <michaelpg@chromium.org> Date: Wed Sep 06 19:23:30 2017 Create TestEventRouter for extensions tests Several extension API tests, and more that I'll be writing, create an EventRouter subclass for one-off testing. This CL combines these classes into one shared test class. Bug: 723754 Change-Id: Id257ba850419ab11c2ca61e1ca0282a806db269c Reviewed-on: https://chromium-review.googlesource.com/647692 Commit-Queue: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org> Reviewed-by: Toni Barzic <tbarzic@chromium.org> Cr-Commit-Position: refs/heads/master@{#500038} [modify] https://crrev.com/2f2cf6cddff6f31b6a3ba55655462a34fb32113f/chrome/browser/chromeos/lock_screen_apps/app_manager_impl_unittest.cc [modify] https://crrev.com/2f2cf6cddff6f31b6a3ba55655462a34fb32113f/chrome/browser/extensions/api/easy_unlock_private/easy_unlock_private_api_chromeos_unittest.cc [modify] https://crrev.com/2f2cf6cddff6f31b6a3ba55655462a34fb32113f/chrome/browser/extensions/api/image_writer_private/operation_manager_unittest.cc [modify] https://crrev.com/2f2cf6cddff6f31b6a3ba55655462a34fb32113f/chrome/browser/signin/easy_unlock_app_manager_unittest.cc [modify] https://crrev.com/2f2cf6cddff6f31b6a3ba55655462a34fb32113f/extensions/BUILD.gn [add] https://crrev.com/2f2cf6cddff6f31b6a3ba55655462a34fb32113f/extensions/browser/test_event_router.cc [add] https://crrev.com/2f2cf6cddff6f31b6a3ba55655462a34fb32113f/extensions/browser/test_event_router.h
Comment 1 by bugdroid1@chromium.org
, May 19 2017