Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(512)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 25366003: Moved some functions off ExtensionService into a new file extension_util. (Closed)

Created:
7 years, 2 months ago by benwells
Modified:
7 years, 2 months ago
Reviewers:
Lei Zhang, Yoyo Zhou, sky
CC:
chromium-reviews, extensions-reviews_chromium.org, vandebo (ex-Chrome), Lei Zhang, tommycli, robertshield, Greg Billock, chromium-apps-reviews_chromium.org, James Su, chrome-apps-syd-reviews_chromium.org
Visibility:
Public.

Description

Moved some functions off ExtensionService into a new file extension_util. These functions do not need any internal state of ExtensionService, so do not need to be on the class itself. This makes it easier to understand what ExtensionService is doing (as it now does less) and easier to change. TBR=sky@chromium.org BUG=298537 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=229223

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Windows compile #

Total comments: 16

Patch Set 4 : Rebase #

Patch Set 5 : Feedback #

Patch Set 6 : Compile failures #

Unified diffs Side-by-side diffs Delta from patch set Stats (+331 lines, -229 lines) Patch
M chrome/browser/autocomplete/extension_app_provider.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/autocomplete/keyword_provider.cc View 3 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/automation/testing_automation_provider.cc View 3 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/chromeos/file_manager/file_browser_handlers.cc View 1 2 3 4 5 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/chromeos/file_manager/file_tasks.cc View 1 2 3 4 5 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc View 1 2 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/developer_private/developer_private_api.cc View 1 2 3 4 chunks +7 lines, -4 lines 0 comments Download
M chrome/browser/extensions/api/media_galleries_private/media_galleries_private_api.cc View 1 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/messaging/message_service.cc View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/module/module.cc View 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/preference/preference_helpers.cc View 3 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/tabs/windows_event_router.cc View 1 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/extensions/context_menu_matcher.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/event_router.cc View 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_browsertest.cc View 1 2 3 chunks +5 lines, -4 lines 0 comments Download
M chrome/browser/extensions/extension_function_dispatcher.cc View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_functional_browsertest.cc View 1 3 chunks +15 lines, -7 lines 0 comments Download
M chrome/browser/extensions/extension_prefs.h View 1 2 3 4 1 chunk +6 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_prefs.cc View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_process_manager.cc View 1 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_service.h View 1 2 3 4 chunks +1 line, -23 lines 0 comments Download
M chrome/browser/extensions/extension_service.cc View 1 2 3 4 8 chunks +12 lines, -121 lines 0 comments Download
M chrome/browser/extensions/extension_service_unittest.cc View 1 2 3 15 chunks +18 lines, -15 lines 0 comments Download
M chrome/browser/extensions/extension_startup_browsertest.cc View 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_system.cc View 1 2 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_toolbar_model.cc View 3 chunks +3 lines, -2 lines 0 comments Download
A chrome/browser/extensions/extension_util.h View 1 2 3 4 1 chunk +49 lines, -0 lines 0 comments Download
A chrome/browser/extensions/extension_util.cc View 1 2 3 4 1 chunk +124 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_web_ui.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/user_script_master.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/sync/test/integration/sync_extension_helper.cc View 1 2 3 4 5 4 chunks +9 lines, -8 lines 0 comments Download
M chrome/browser/ui/app_list/search/app_search_provider.cc View 1 2 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm View 1 2 3 4 5 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/ui/gtk/browser_actions_toolbar_gtk.cc View 1 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/browser_actions_container.cc View 1 2 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/ui/webui/extensions/extension_settings_handler.cc View 1 2 4 chunks +8 lines, -5 lines 0 comments Download
M chrome/chrome_browser_extensions.gypi View 1 2 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
benwells
Not ready yet, but your opinion would be welcome. Does this look right? Alternatively it ...
7 years, 2 months ago (2013-10-15 06:32:53 UTC) #1
Yoyo Zhou
LGTM https://chromiumcodereview.appspot.com/25366003/diff/9001/chrome/browser/extensions/extension_prefs.h File chrome/browser/extensions/extension_prefs.h (right): https://chromiumcodereview.appspot.com/25366003/diff/9001/chrome/browser/extensions/extension_prefs.h#newcode372 chrome/browser/extensions/extension_prefs.h:372: // IMPORTANT: you probably want to use ExtensionService::IsIncognitoEnabled ...
7 years, 2 months ago (2013-10-15 21:25:48 UTC) #2
benwells
+tbr sky for refactoring fallout https://codereview.chromium.org/25366003/diff/9001/chrome/browser/extensions/extension_prefs.h File chrome/browser/extensions/extension_prefs.h (right): https://codereview.chromium.org/25366003/diff/9001/chrome/browser/extensions/extension_prefs.h#newcode372 chrome/browser/extensions/extension_prefs.h:372: // IMPORTANT: you probably ...
7 years, 2 months ago (2013-10-17 06:56:54 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/25366003/24001
7 years, 2 months ago (2013-10-17 06:59:08 UTC) #4
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 2 months ago (2013-10-17 12:05:04 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/25366003/24001
7 years, 2 months ago (2013-10-17 18:30:38 UTC) #6
commit-bot: I haz the power
7 years, 2 months ago (2013-10-17 20:56:07 UTC) #7
Message was sent while issue was closed. 
Change committed as 229223

Powered by Google App Engine
This is Rietveld 408576698