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

Issue 12224027: Revert "GTTF: Add missing virtual destructors." (Closed)

Created:
7 years, 10 months ago by Paweł Hajdan Jr.
Modified:
7 years, 10 months ago
CC:
chromium-reviews, MAD, jam, sadrul, dhollowa+watch_chromium.org, rpetterson, ahutter, browser-components-watch_chromium.org, dcheng, Dmitry Titov, ajwong+watch_chromium.org, markusheintz_, Ilya Sherman, cbentzel+watch_chromium.org, benquan, jar (doing other things), ben+watch_chromium.org, benjhayden+dwatch_chromium.org, achuith+watch_chromium.org, dbeam+watch-autofill_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, groby+spellwatch_chromium.org, chromium-apps-reviews_chromium.org, erikwright+watch_chromium.org, Avi (use Gerrit), jennb, creis+watch_chromium.org, dyu1, rouslan+spellwatch_chromium.org, jianli, feature-media-reviews_chromium.org, pam+watch_chromium.org, Albert Bodenhamer, Raman Kakilate, sail+watch_chromium.org, Aaron Boodman, Dane Wallinga, rdsmith+dwatch_chromium.org, estade+watch_chromium.org, cc-bugs_chromium.org
Visibility:
Public.

Description

Revert "GTTF: Add missing virtual destructors." It turned out this is not necessary, we have a better warning already enabled in clang (-Wdelete-non-virtual-dtor, part of -Wall). TBR=darin,satorux,fischman,jamesr,jar,sky BUG=45135 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=180971

Patch Set 1 #

Patch Set 2 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -119 lines) Patch
M ash/wm/system_modal_container_event_filter_delegate.h View 1 chunk +0 lines, -2 lines 0 comments Download
M base/debug/trace_event_impl.h View 1 chunk +0 lines, -2 lines 0 comments Download
M base/prefs/pref_observer.h View 1 chunk +0 lines, -2 lines 0 comments Download
M base/prefs/public/pref_change_registrar.h View 1 chunk +1 line, -2 lines 0 comments Download
M base/prefs/public/pref_member.h View 1 chunk +1 line, -2 lines 0 comments Download
M base/threading/post_task_and_reply_impl.h View 1 chunk +0 lines, -2 lines 0 comments Download
M cc/layer_animation_event_observer.h View 1 chunk +0 lines, -3 lines 0 comments Download
M cc/layer_tree_host.h View 2 chunks +2 lines, -2 lines 0 comments Download
M cc/layer_tree_host_impl.h View 2 chunks +1 line, -4 lines 0 comments Download
M cc/picture_layer_impl.h View 1 chunk +2 lines, -3 lines 0 comments Download
M cc/picture_layer_tiling.h View 1 chunk +0 lines, -3 lines 0 comments Download
M cc/rate_limiter.h View 1 chunk +0 lines, -3 lines 0 comments Download
M cc/video_frame_provider.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/certificate_manager_model.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/download/download_danger_prompt.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/extensions/activity_log.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/developer_private/entry_picker.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/page_capture/page_capture_api.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/extensions/app_notify_channel_setup.h View 2 chunks +0 lines, -4 lines 0 comments Download
M chrome/browser/extensions/event_router.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_function.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_keybinding_registry.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_scoped_prefs.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_warning_service.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/favicon/favicon_handler_delegate.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/google_apis/auth_service_interface.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/managed_mode/managed_mode_url_filter.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/metrics/variations/resource_request_allowed_notifier.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/policy/user_info_fetcher.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/profiles/profile.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/signin/about_signin_internals.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/signin/signin_internals_util.h View 2 chunks +1 line, -2 lines 0 comments Download
M chrome/browser/signin/signin_tracker.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/spellchecker/spellcheck_custom_dictionary.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/spellchecker/spellcheck_hunspell_dictionary.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/tab_contents/render_view_context_menu.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/app_modal_dialogs/native_app_modal_dialog.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/autofill/autofill_popup_delegate.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/base_window.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/gtk/avatar_menu_item_gtk.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/ui/panels/display_settings_provider.h View 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/ui/website_settings/permission_menu_model.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/utility/chrome_content_utility_client.h View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/download/download_item_impl.h View 1 chunk +1 line, -1 line 0 comments Download
M content/public/browser/download_destination_observer.h View 1 chunk +0 lines, -2 lines 0 comments Download
M content/public/utility/content_utility_client.h View 1 chunk +0 lines, -2 lines 0 comments Download
M dbus/property.h View 1 chunk +0 lines, -1 line 0 comments Download
M ipc/ipc_channel_proxy.h View 1 chunk +0 lines, -2 lines 0 comments Download
M media/audio/audio_manager.h View 1 chunk +0 lines, -2 lines 0 comments Download
M media/filters/ffmpeg_glue.h View 1 chunk +0 lines, -2 lines 0 comments Download
M net/http/http_pipelined_connection.h View 1 chunk +0 lines, -2 lines 0 comments Download
M net/http/http_pipelined_host.h View 1 chunk +0 lines, -2 lines 0 comments Download
M net/http/http_pipelined_host_forced.h View 1 chunk +1 line, -1 line 0 comments Download
M net/http/http_pipelined_host_impl.h View 1 chunk +1 line, -1 line 0 comments Download
M net/http/http_pipelined_host_pool.h View 1 chunk +1 line, -3 lines 0 comments Download
M net/http/http_stream_factory_impl.h View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 2 (0 generated)
Paweł Hajdan Jr.
TBR FYI, I have reverted the patch after discussion on the bug.
7 years, 10 months ago (2013-02-06 18:14:58 UTC) #1
darin (slow to review)
7 years, 10 months ago (2013-02-06 18:23:31 UTC) #2
LGTM

On Wed, Feb 6, 2013 at 10:14 AM, <phajdan.jr@chromium.org> wrote:

> Reviewers: jar, jamesr, darin, satorux1, Ami Fischman, sky,
>
> Message:
> TBR
>
> FYI, I have reverted the patch after discussion on the bug.
>
> Description:
> Revert "GTTF: Add missing virtual destructors."
>
> It turned out this is not necessary,
> we have a better warning already enabled in clang
> (-Wdelete-non-virtual-dtor, part of -Wall).
>
> TBR=darin,satorux,fischman,**jamesr,jar,sky
>
> BUG=45135
>
> Committed: https://src.chromium.org/**viewvc/chrome?view=rev&**
>
revision=180971<https://src.chromium.org/viewvc/chrome?view=rev&revision=180971>
>
> Please review this at
https://codereview.chromium.**org/12224027/<https://codereview.chromium.org/1...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>   M ash/wm/system_modal_container_**event_filter_delegate.h
>   M base/debug/trace_event_impl.h
>   M base/prefs/pref_observer.h
>   M base/prefs/public/pref_change_**registrar.h
>   M base/prefs/public/pref_member.**h
>   M base/threading/post_task_and_**reply_impl.h
>   M cc/layer_animation_event_**observer.h
>   M cc/layer_tree_host.h
>   M cc/layer_tree_host_impl.h
>   M cc/picture_layer_impl.h
>   M cc/picture_layer_tiling.h
>   M cc/rate_limiter.h
>   M cc/video_frame_provider.h
>   M chrome/browser/certificate_**manager_model.h
>   M chrome/browser/download/**download_danger_prompt.h
>   M chrome/browser/extensions/**activity_log.h
>   M chrome/browser/extensions/api/**developer_private/entry_**picker.h
>   M chrome/browser/extensions/api/**page_capture/page_capture_api.**h
>   M chrome/browser/extensions/app_**notify_channel_setup.h
>   M chrome/browser/extensions/**event_router.h
>   M chrome/browser/extensions/**extension_function.h
>   M chrome/browser/extensions/**extension_keybinding_registry.**h
>   M chrome/browser/extensions/**extension_scoped_prefs.h
>   M chrome/browser/extensions/**extension_warning_service.h
>   M chrome/browser/favicon/**favicon_handler_delegate.h
>   M chrome/browser/google_apis/**auth_service_interface.h
>   M chrome/browser/managed_mode/**managed_mode_url_filter.h
>   M chrome/browser/metrics/**variations/resource_request_**
> allowed_notifier.h
>   M chrome/browser/policy/user_**info_fetcher.h
>   M chrome/browser/profiles/**profile.h
>   M chrome/browser/signin/about_**signin_internals.h
>   M chrome/browser/signin/signin_**internals_util.h
>   M chrome/browser/signin/signin_**tracker.h
>   M chrome/browser/spellchecker/**spellcheck_custom_dictionary.h
>   M chrome/browser/spellchecker/**spellcheck_hunspell_**dictionary.h
>   M chrome/browser/tab_contents/**render_view_context_menu.h
>   M chrome/browser/ui/app_modal_**dialogs/native_app_modal_**dialog.h
>   M chrome/browser/ui/autofill/**autofill_popup_delegate.h
>   M chrome/browser/ui/base_window.**h
>   M chrome/browser/ui/gtk/avatar_**menu_item_gtk.h
>   M chrome/browser/ui/panels/**display_settings_provider.h
>   M chrome/browser/ui/website_**settings/permission_menu_**model.h
>   M chrome/utility/chrome_content_**utility_client.h
>   M content/browser/download/**download_item_impl.h
>   M content/public/browser/**download_destination_observer.**h
>   M content/public/utility/**content_utility_client.h
>   M dbus/property.h
>   M ipc/ipc_channel_proxy.h
>   M media/audio/audio_manager.h
>   M media/filters/ffmpeg_glue.h
>   M net/http/http_pipelined_**connection.h
>   M net/http/http_pipelined_host.h
>   M net/http/http_pipelined_host_**forced.h
>   M net/http/http_pipelined_host_**impl.h
>   M net/http/http_pipelined_host_**pool.h
>   M net/http/http_stream_factory_**impl.h
>
>
>

Powered by Google App Engine
This is Rietveld 408576698