New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 616214 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Making RenderFrameObserver::OnDestruct() and RenderViewObserver::OnDestruct() pure virtual.

Project Member Reported by x...@chromium.org, May 31 2016

Issue description

The default behavior of these two methods is deleting the observer while the RenderFrame/RenderView goes away. As developers are generally not aware of that simply inheriting the observer interface means transferring the ownership to RenderFrame/RenderView, this can cause "mysterious crash" and is very difficult to debug. For safety, we should make OnDestruct() a pure virtual method in RFObserver/RenderView. All classes must implement the method, even if it should be empty. This would ensure the compiler makes a developer aware of the lifecycle concerns around RenderFrame. In the header file, document that OnDestruct() can be used by a subclass to delete itself, but if it does not, then the subclass must always null-check each call to RFO::render_frame()/RVO::render_view() because the RenderFrame/RenderView can go away at any time.
 

Comment 1 by x...@chromium.org, Jun 3 2016

Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 7 2016

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

commit 694b50a9d759601471f5b8c8d61bd488b6a4cfba
Author: xjz <xjz@chromium.org>
Date: Tue Jun 07 21:49:37 2016

Makes RenderFrameObserver/RenderViewObserver::OnDestruct pure virtual.
All subclasses must implement the method, even if it should be empty.
This would ensure the compiler makes a developer aware of the lifecycle
concerns around RenderFrame/RenderView, and avoid the unintended
transfering ownership when inheriting the observer interface.

BUG= 616214 

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

[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/aw_content_settings_client.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/aw_content_settings_client.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/aw_message_port_client.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/aw_message_port_client.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/aw_render_frame_ext.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/aw_render_frame_ext.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/aw_render_view_ext.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/aw_render_view_ext.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/print_render_frame_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/android_webview/renderer/print_render_frame_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/banners/app_banner_client.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/banners/app_banner_client.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/chrome_render_frame_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/chrome_render_frame_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/chrome_render_view_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/chrome_render_view_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/content_settings_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/content_settings_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/net/net_error_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/net/net_error_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/page_load_histograms.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/page_load_histograms.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/pepper/pepper_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/pepper/pepper_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/plugins/plugin_preroller.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/plugins/plugin_preroller.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/prerender/prerender_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/prerender/prerender_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/prerender/prerenderer_client.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/prerender/prerenderer_client.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/safe_browsing/phishing_classifier_delegate.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/safe_browsing/phishing_classifier_delegate.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/safe_browsing/threat_dom_details.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/safe_browsing/threat_dom_details.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/searchbox/searchbox.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/searchbox/searchbox.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/spellchecker/spellcheck_provider.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chrome/renderer/spellchecker/spellcheck_provider.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chromecast/renderer/cast_media_load_deferrer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/chromecast/renderer/cast_media_load_deferrer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/autofill/content/renderer/page_click_tracker.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/autofill/content/renderer/page_click_tracker.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/contextual_search/renderer/overlay_js_render_frame_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/contextual_search/renderer/overlay_js_render_frame_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/dom_distiller/content/renderer/distillability_agent.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/dom_distiller/content/renderer/distillability_agent.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/dom_distiller/content/renderer/distiller_js_render_frame_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/dom_distiller/content/renderer/distiller_js_render_frame_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/nacl/renderer/nacl_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/nacl/renderer/nacl_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/page_load_metrics/renderer/metrics_render_frame_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/page_load_metrics/renderer/metrics_render_frame_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/password_manager/content/renderer/credential_manager_client.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/password_manager/content/renderer/credential_manager_client.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/plugins/renderer/webview_plugin.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/printing/renderer/print_web_view_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/printing/renderer/print_web_view_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/translate/content/renderer/translate_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/components/translate/content/renderer/translate_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/public/renderer/render_frame_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/public/renderer/render_frame_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/public/renderer/render_view_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/public/renderer/render_view_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/public/test/frame_load_waiter.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/public/test/frame_load_waiter.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/accessibility/renderer_accessibility.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/accessibility/renderer_accessibility.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/android/renderer_date_time_picker.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/android/renderer_date_time_picker.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/devtools/devtools_agent.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/devtools/devtools_agent.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/devtools/devtools_client.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/devtools/devtools_client.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/dom_serializer_browsertest.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/idle_user_detector.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/idle_user_detector.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/image_downloader/image_downloader_impl.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/image_downloader/image_downloader_impl.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/java/gin_java_bridge_dispatcher.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/java/gin_java_bridge_dispatcher.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/manifest/manifest_manager.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/manifest/manifest_manager.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/android/renderer_media_player_manager.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/android/renderer_media_player_manager.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/android/renderer_media_session_manager.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/android/renderer_media_session_manager.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/android/renderer_surface_view_manager.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/android/renderer_surface_view_manager.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/cdm/renderer_cdm_manager.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/cdm/renderer_cdm_manager.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/media_stream_video_capturer_source.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/midi_dispatcher.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/midi_dispatcher.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/renderer_webmediaplayer_delegate.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/renderer_webmediaplayer_delegate.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/user_media_client_impl.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/media/user_media_client_impl.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/mojo_bindings_controller.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/mojo_bindings_controller.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/notification_permission_dispatcher.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/notification_permission_dispatcher.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/pepper/pepper_browser_connection.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/pepper/pepper_browser_connection.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/pepper/pepper_media_device_manager.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/pepper/pepper_media_device_manager.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/pepper/plugin_power_saver_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/pepper/plugin_power_saver_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/presentation/presentation_dispatcher.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/presentation/presentation_dispatcher.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/push_messaging/push_messaging_dispatcher.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/push_messaging/push_messaging_dispatcher.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/render_frame_impl_browsertest.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/render_view_mouse_lock_dispatcher.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/render_view_mouse_lock_dispatcher.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/screen_orientation/screen_orientation_dispatcher.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/screen_orientation/screen_orientation_dispatcher.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/shared_worker_repository.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/shared_worker_repository.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/speech_recognition_dispatcher.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/speech_recognition_dispatcher.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/stats_collection_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/stats_collection_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/text_input_client_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/text_input_client_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/visual_state_browsertest.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/web_ui_extension_data.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/renderer/web_ui_extension_data.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/shell/renderer/layout_test/blink_test_runner.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/shell/renderer/layout_test/blink_test_runner.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/shell/renderer/layout_test/layout_test_render_frame_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/shell/renderer/layout_test/layout_test_render_frame_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/shell/renderer/shell_render_view_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/content/shell/renderer/shell_render_view_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/extensions/renderer/api/automation/automation_api_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/extensions/renderer/api/automation/automation_api_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/extensions/renderer/extension_frame_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/extensions/renderer/extension_frame_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/extensions/renderer/extension_helper.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/extensions/renderer/extension_helper.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/extensions/renderer/extensions_render_frame_observer.cc
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/extensions/renderer/extensions_render_frame_observer.h
[modify] https://crrev.com/694b50a9d759601471f5b8c8d61bd488b6a4cfba/extensions/renderer/render_frame_observer_natives.cc

Comment 3 by x...@chromium.org, Jun 7 2016

Status: Fixed (was: Started)

Sign in to add a comment