Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(46)

Issue 6249010: Cleanup: de-inline a bunch of classes, rename and move "PluginInstaller" to "... (Closed)

Created:
9 years, 11 months ago by Peter Kasting
Modified:
9 years, 7 months ago
CC:
chromium-reviews, James Hawkins, Erik does not do reviews, brettw-cc_chromium.org, jam, Aaron Boodman, pam+watch_chromium.org, rdsmith+dwatch_chromium.org, Paweł Hajdan Jr., darin-cc_chromium.org, Ilya Sherman, stuartmorgan+watch_chromium.org, dhollowa
Visibility:
Public.

Description

Cleanup: de-inline a bunch of classes, rename and move "PluginInstaller" to "PluginInstallerInfoBarDelegate" for clarity, lots of other misc. stuff BUG=none TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72166

Patch Set 1 #

Total comments: 4

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Patch Set 6 : '' #

Patch Set 7 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2024 lines, -1994 lines) Patch
M chrome/browser/alternate_nav_url_fetcher.h View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/alternate_nav_url_fetcher.cc View 1 2 3 4 5 6 2 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/autofill/autofill_cc_infobar_delegate.h View 1 2 3 4 5 6 3 chunks +6 lines, -8 lines 0 comments Download
M chrome/browser/autofill/autofill_cc_infobar_delegate.cc View 1 2 3 4 5 6 4 chunks +12 lines, -23 lines 0 comments Download
M chrome/browser/automation/automation_provider.cc View 1 2 3 4 5 6 2 chunks +1 line, -3 lines 0 comments Download
M chrome/browser/automation/automation_provider_observers.cc View 1 2 3 4 5 6 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/automation/testing_automation_provider.h View 1 2 3 4 5 6 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/automation/testing_automation_provider.cc View 1 2 3 4 5 6 4 chunks +9 lines, -11 lines 0 comments Download
M chrome/browser/download/download_manager.cc View 1 2 3 4 5 6 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/download/download_request_infobar_delegate.h View 1 2 3 4 5 6 2 chunks +9 lines, -14 lines 0 comments Download
M chrome/browser/download/download_request_infobar_delegate.cc View 1 2 3 4 5 6 3 chunks +10 lines, -11 lines 0 comments Download
M chrome/browser/download/download_request_infobar_delegate_unittest.cc View 1 2 3 4 5 6 3 chunks +45 lines, -33 lines 0 comments Download
M chrome/browser/download/download_util.cc View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/extensions/crashed_extension_infobar.h View 1 2 3 4 5 6 2 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/extensions/crashed_extension_infobar.cc View 1 2 3 4 5 6 4 chunks +16 lines, -15 lines 0 comments Download
M chrome/browser/extensions/extension_crash_recovery_browsertest.cc View 1 2 3 4 5 6 10 chunks +18 lines, -22 lines 0 comments Download
M chrome/browser/extensions/extension_disabled_infobar_delegate.cc View 1 2 3 4 5 6 3 chunks +119 lines, -83 lines 0 comments Download
M chrome/browser/extensions/extension_dom_ui.h View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_dom_ui.cc View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_function_dispatcher.h View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_host.h View 1 2 3 4 5 6 3 chunks +11 lines, -8 lines 0 comments Download
M chrome/browser/extensions/extension_host.cc View 1 2 3 4 5 6 3 chunks +7 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_infobar_delegate.h View 1 2 3 4 5 6 3 chunks +15 lines, -14 lines 0 comments Download
M chrome/browser/extensions/extension_infobar_delegate.cc View 1 2 3 4 5 6 3 chunks +10 lines, -10 lines 0 comments Download
M chrome/browser/extensions/extension_install_ui.cc View 1 2 3 4 5 6 3 chunks +13 lines, -12 lines 0 comments Download
M chrome/browser/extensions/extension_install_ui_browsertest.cc View 1 2 3 4 5 6 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_tabs_module.cc View 1 2 3 4 5 6 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/extensions/theme_installed_infobar_delegate.h View 1 2 3 4 5 6 2 chunks +18 lines, -13 lines 0 comments Download
M chrome/browser/extensions/theme_installed_infobar_delegate.cc View 1 2 3 4 5 6 5 chunks +30 lines, -33 lines 0 comments Download
M chrome/browser/external_tab_container_win.h View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/external_tab_container_win.cc View 1 2 3 4 5 6 1 chunk +2 lines, -3 lines 0 comments Download
M chrome/browser/geolocation/geolocation_permission_context.cc View 1 2 3 4 5 6 19 chunks +188 lines, -139 lines 0 comments Download
MM chrome/browser/geolocation/geolocation_permission_context_unittest.cc View 1 2 3 4 5 6 5 chunks +9 lines, -17 lines 0 comments Download
M chrome/browser/google/google_url_tracker_unittest.cc View 1 2 3 4 5 6 2 chunks +6 lines, -5 lines 0 comments Download
chrome/browser/notifications/balloon_host.h View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/notifications/balloon_host.cc View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/notifications/desktop_notification_service.cc View 1 2 3 4 5 6 3 chunks +162 lines, -118 lines 0 comments Download
M chrome/browser/omnibox_search_hint.cc View 1 2 3 4 5 6 3 chunks +85 lines, -62 lines 0 comments Download
M chrome/browser/password_manager_delegate_impl.cc View 1 2 3 4 5 6 2 chunks +75 lines, -58 lines 0 comments Download
D chrome/browser/plugin_installer.h View 1 2 3 4 5 6 1 chunk +0 lines, -40 lines 0 comments Download
D chrome/browser/plugin_installer.cc View 1 2 3 4 5 6 1 chunk +0 lines, -83 lines 0 comments Download
A + chrome/browser/plugin_installer_infobar_delegate.h View 3 chunks +10 lines, -12 lines 0 comments Download
A + chrome/browser/plugin_installer_infobar_delegate.cc View 3 chunks +23 lines, -24 lines 0 comments Download
M chrome/browser/printing/print_dialog_gtk.cc View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/renderer_host/render_view_host.h View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/speech/speech_input_manager.cc View 1 2 3 4 5 6 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/tab_contents/infobar_delegate.h View 1 2 3 4 5 6 9 chunks +44 lines, -60 lines 0 comments Download
M chrome/browser/tab_contents/infobar_delegate.cc View 1 2 3 4 5 6 6 chunks +63 lines, -58 lines 0 comments Download
M chrome/browser/tab_contents/tab_contents.h View 1 2 3 4 5 6 3 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/tab_contents/tab_contents.cc View 1 2 3 4 5 6 9 chunks +96 lines, -104 lines 0 comments Download
M chrome/browser/tab_contents/tab_contents_ssl_helper.h View 1 2 3 4 5 6 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/tab_contents/tab_contents_ssl_helper.cc View 1 2 3 4 5 6 2 chunks +116 lines, -98 lines 0 comments Download
M chrome/browser/task_manager/task_manager_browsertest.cc View 1 2 3 4 5 6 2 chunks +5 lines, -6 lines 0 comments Download
M chrome/browser/translate/languages_menu_model.cc View 1 2 3 4 5 6 2 chunks +8 lines, -8 lines 0 comments Download
MM chrome/browser/translate/translate_infobar_delegate.h View 1 2 3 4 5 6 5 chunks +11 lines, -11 lines 0 comments Download
M chrome/browser/translate/translate_infobar_delegate.cc View 1 2 3 4 5 6 14 chunks +141 lines, -173 lines 0 comments Download
M chrome/browser/translate/translate_manager.cc View 1 2 3 4 5 6 3 chunks +4 lines, -8 lines 0 comments Download
M chrome/browser/translate/translate_manager_unittest.cc View 1 2 3 4 5 6 7 chunks +15 lines, -16 lines 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 4 5 6 1 chunk +6 lines, -9 lines 0 comments Download
M chrome/browser/ui/browser_init.cc View 1 2 3 4 5 6 5 chunks +213 lines, -126 lines 0 comments Download
M chrome/browser/ui/cocoa/infobars/infobar_controller_unittest.mm View 1 2 3 4 5 6 3 chunks +51 lines, -51 lines 0 comments Download
M chrome/browser/ui/cocoa/infobars/infobar_test_helper.h View 1 2 3 4 5 6 1 chunk +87 lines, -129 lines 0 comments Download
A + chrome/browser/ui/cocoa/infobars/infobar_test_helper.mm View 1 2 3 4 5 1 chunk +111 lines, -132 lines 0 comments Download
M chrome/browser/ui/cocoa/keystone_infobar.mm View 1 2 3 4 5 6 2 chunks +74 lines, -61 lines 0 comments Download
M chrome/browser/ui/cocoa/translate/translate_infobar_unittest.mm View 1 2 3 4 5 6 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/gtk/translate/translate_infobar_base_gtk.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/frame/browser_view.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/frame/browser_view.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/infobars/infobar_container.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/infobars/infobar_container.cc View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/infobars/infobars.cc View 1 2 3 4 5 6 1 chunk +6 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/infobars/translate_infobar_base.cc View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/common/automation_messages_internal.h View 1 2 3 4 5 6 3 chunks +1 line, -3 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Peter Kasting
There shouldn't be any functional changes in this.
9 years, 11 months ago (2011-01-20 00:11:15 UTC) #1
Elliot Glaysher
LGTM++ http://codereview.chromium.org/6249010/diff/1/chrome/browser/notifications/desktop_notification_service.cc File chrome/browser/notifications/desktop_notification_service.cc (right): http://codereview.chromium.org/6249010/diff/1/chrome/browser/notifications/desktop_notification_service.cc#newcode99 chrome/browser/notifications/desktop_notification_service.cc:99: ~NotificationPermissionInfoBarDelegate(); virtual, I presume? http://codereview.chromium.org/6249010/diff/1/chrome/browser/tab_contents/tab_contents.cc File chrome/browser/tab_contents/tab_contents.cc (left): ...
9 years, 11 months ago (2011-01-20 01:02:03 UTC) #2
Peter Kasting
Will submit once I can get clean(ish) try runs. http://codereview.chromium.org/6249010/diff/1/chrome/browser/notifications/desktop_notification_service.cc File chrome/browser/notifications/desktop_notification_service.cc (right): http://codereview.chromium.org/6249010/diff/1/chrome/browser/notifications/desktop_notification_service.cc#newcode99 chrome/browser/notifications/desktop_notification_service.cc:99: ...
9 years, 11 months ago (2011-01-20 01:45:14 UTC) #3
Bernhard Bauer
Drive-by question: Is there a particular advantage to de-inlining classes defined in .cc files? Is ...
9 years, 11 months ago (2011-01-28 12:23:55 UTC) #4
Peter Kasting
9 years, 11 months ago (2011-01-28 17:18:16 UTC) #5
On 2011/01/28 12:23:55, Bernhard Bauer wrote:
> Drive-by question: Is there a particular advantage to de-inlining classes
> defined in .cc files? Is this preferred now?

It is preferred; from http://dev.chromium.org/developers/coding-style : "Even
with a class that's local to a single .cc file, declaring and defining things
separately is a good idea, and keeps the declaration more readable.", and the
other notes about inline functions there also apply.

During my infobar rewrite I've found that major changes made to de-inlined class
definitions are generally easier to understand, since one can see API changes
and function-body changes separately.

Powered by Google App Engine
This is Rietveld 408576698