Skip to content

Commit

Permalink
Makes RenderFrameObserver/RenderViewObserver::OnDestruct pure virtual.
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
xjz authored and Commit bot committed Jun 7, 2016
1 parent b9e16a7 commit 694b50a
Show file tree
Hide file tree
Showing 143 changed files with 405 additions and 31 deletions.
4 changes: 4 additions & 0 deletions android_webview/renderer/aw_content_settings_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,8 @@ bool AwContentSettingsClient::allowRunningInsecureContent(
return enabled_per_settings ? true : AllowMixedContent(url);
}

void AwContentSettingsClient::OnDestruct() {
delete this;
}

} // namespace android_webview
3 changes: 3 additions & 0 deletions android_webview/renderer/aw_content_settings_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ class AwContentSettingsClient : public content::RenderFrameObserver,
private:
~AwContentSettingsClient() override;

// content::RenderFrameObserver implementation.
void OnDestruct() override;

// blink::WebContentSettingsClient implementation.
bool allowDisplayingInsecureContent(
bool enabled_per_settings,
Expand Down
4 changes: 4 additions & 0 deletions android_webview/renderer/aw_message_port_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ bool AwMessagePortClient::OnMessageReceived(
return handled;
}

void AwMessagePortClient::OnDestruct() {
delete this;
}

void AwMessagePortClient::OnWebToAppMessage(
int message_port_id,
const base::string16& message,
Expand Down
1 change: 1 addition & 0 deletions android_webview/renderer/aw_message_port_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class AwMessagePortClient : public content::RenderFrameObserver {

// RenderFrameObserver
bool OnMessageReceived(const IPC::Message& message) override;
void OnDestruct() override;

void OnWebToAppMessage(int message_port_id,
const base::string16& message,
Expand Down
4 changes: 4 additions & 0 deletions android_webview/renderer/aw_render_frame_ext.cc
Original file line number Diff line number Diff line change
Expand Up @@ -302,4 +302,8 @@ blink::WebFrameWidget* AwRenderFrameExt::GetWebFrameWidget() {
return render_frame()->GetRenderView()->GetWebFrameWidget();
}

void AwRenderFrameExt::OnDestruct() {
delete this;
}

} // namespace android_webview
1 change: 1 addition & 0 deletions android_webview/renderer/aw_render_frame_ext.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class AwRenderFrameExt : public content::RenderFrameObserver {

bool OnMessageReceived(const IPC::Message& message) override;
void FocusedNodeChanged(const blink::WebNode& node) override;
void OnDestruct() override;

void OnDocumentHasImagesRequest(uint32_t id);
void OnDoHitTest(const gfx::PointF& touch_center,
Expand Down
4 changes: 4 additions & 0 deletions android_webview/renderer/aw_render_view_ext.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ void AwRenderViewExt::DidUpdateLayout() {
PostCheckContentsSize();
}

void AwRenderViewExt::OnDestruct() {
delete this;
}

void AwRenderViewExt::PostCheckContentsSize() {
if (check_contents_size_timer_.IsRunning())
return;
Expand Down
1 change: 1 addition & 0 deletions android_webview/renderer/aw_render_view_ext.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class AwRenderViewExt : public content::RenderViewObserver {
// RenderViewObserver:
void DidCommitCompositorFrame() override;
void DidUpdateLayout() override;
void OnDestruct() override;

void CheckContentsSize();
void PostCheckContentsSize();
Expand Down
4 changes: 4 additions & 0 deletions android_webview/renderer/print_render_frame_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,8 @@ void PrintRenderFrameObserver::OnPrintNodeUnderContextMenu() {
helper->PrintNode(render_frame()->GetWebFrame()->contextMenuNode());
}

void PrintRenderFrameObserver::OnDestruct() {
delete this;
}

} // namespace android_webview
1 change: 1 addition & 0 deletions android_webview/renderer/print_render_frame_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class PrintRenderFrameObserver : public content::RenderFrameObserver {

// RenderFrameObserver implementation.
bool OnMessageReceived(const IPC::Message& message) override;
void OnDestruct() override;

// IPC handlers
void OnPrintNodeUnderContextMenu();
Expand Down
7 changes: 1 addition & 6 deletions chrome/renderer/banners/app_banner_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@ AppBannerClient::AppBannerClient(content::RenderFrame* render_frame)
AppBannerClient::~AppBannerClient() {
}

void AppBannerClient::OnDestruct() {
// The RenderFrameImpl destructor calls RenderFrameObserver::OnDestruct, which
// deletes this object. However, RenderFrameImpl also holds a scoped pointer
// to AppBannerClient. Override OnDestruct to be a no-op to stop a double call
// to the destructor and a renderer crash.
}
void AppBannerClient::OnDestruct() {}

bool AppBannerClient::OnMessageReceived(const IPC::Message& message) {
bool handled = true;
Expand Down
3 changes: 1 addition & 2 deletions chrome/renderer/banners/app_banner_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ class AppBannerClient : public content::RenderFrameObserver,
~AppBannerClient() override;

private:
// content::RenderFrame::Observer implementation.
// content::RenderFrameObserver implementation.
void OnDestruct() override;

bool OnMessageReceived(const IPC::Message& message) override;

// WebAppBannerClient implementation.
Expand Down
1 change: 1 addition & 0 deletions chrome/renderer/chrome_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ class MediaLoadDeferrer : public content::RenderFrameObserver {
continue_loading_cb_.Run();
delete this;
}
void OnDestruct() override { delete this; }

const base::Closure continue_loading_cb_;

Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/chrome_render_frame_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -420,3 +420,7 @@ void ChromeRenderFrameObserver::DidMeaningfulLayout(
break;
}
}

void ChromeRenderFrameObserver::OnDestruct() {
delete this;
}
1 change: 1 addition & 0 deletions chrome/renderer/chrome_render_frame_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class ChromeRenderFrameObserver : public content::RenderFrameObserver {
void DidCommitProvisionalLoad(bool is_new_navigation,
bool is_same_page_navigation) override;
void DidMeaningfulLayout(blink::WebMeaningfulLayout layout_type) override;
void OnDestruct() override;

// IPC handlers
void OnSetIsPrerendering(bool is_prerendering);
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/chrome_render_view_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,7 @@ void ChromeRenderViewObserver::DidCommitProvisionalLoad(
webui_javascript_.clear();
}
}

void ChromeRenderViewObserver::OnDestruct() {
delete this;
}
1 change: 1 addition & 0 deletions chrome/renderer/chrome_render_view_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class ChromeRenderViewObserver : public content::RenderViewObserver {
void DidCommitProvisionalLoad(blink::WebLocalFrame* frame,
bool is_new_navigation) override;
void Navigate(const GURL& url) override;
void OnDestruct() override;

#if !defined(OS_ANDROID)
void OnWebUIJavaScript(const base::string16& javascript);
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/content_settings_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ void ContentSettingsObserver::DidCommitProvisionalLoad(
!url.SchemeIs(url::kDataScheme));
}

void ContentSettingsObserver::OnDestruct() {
delete this;
}

bool ContentSettingsObserver::allowDatabase(const WebString& name,
const WebString& display_name,
unsigned long estimated_size) {
Expand Down
1 change: 1 addition & 0 deletions chrome/renderer/content_settings_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class ContentSettingsObserver
bool OnMessageReceived(const IPC::Message& message) override;
void DidCommitProvisionalLoad(bool is_new_navigation,
bool is_same_page_navigation) override;
void OnDestruct() override;

// Message handlers.
void OnLoadBlockedPlugins(const std::string& identifier);
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/net/net_error_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ bool NetErrorHelper::OnMessageReceived(const IPC::Message& message) {
return handled;
}

void NetErrorHelper::OnDestruct() {
delete this;
}

void NetErrorHelper::NetworkStateChanged(bool enabled) {
core_->NetworkStateChanged(enabled);
}
Expand Down
1 change: 1 addition & 0 deletions chrome/renderer/net/net_error_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class NetErrorHelper
void OnStop() override;
void WasShown() override;
void WasHidden() override;
void OnDestruct() override;

// IPC::Listener implementation.
bool OnMessageReceived(const IPC::Message& message) override;
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/page_load_histograms.cc
Original file line number Diff line number Diff line change
Expand Up @@ -967,3 +967,7 @@ void PageLoadHistograms::LogPageLoadTime(const DocumentState* document_state,
VLOG(1) << "PLT: " << (finish - start).InMilliseconds() << "ms "
<< url.spec();
}

void PageLoadHistograms::OnDestruct() {
delete this;
}
1 change: 1 addition & 0 deletions chrome/renderer/page_load_histograms.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class PageLoadHistograms : public content::RenderViewObserver {
// RenderViewObserver implementation.
void FrameWillClose(blink::WebFrame* frame) override;
void ClosePage() override;
void OnDestruct() override;

// Dump all page load histograms appropriate for the given frame.
//
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/pepper/pepper_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,7 @@ void PepperHelper::DidCreatePepperPlugin(content::RendererPpapiHost* host) {
host->GetPpapiHost()->AddInstanceMessageFilter(
base::WrapUnique(new PepperSharedMemoryMessageFilter(host)));
}

void PepperHelper::OnDestruct() {
delete this;
}
1 change: 1 addition & 0 deletions chrome/renderer/pepper/pepper_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class PepperHelper : public content::RenderFrameObserver {

// RenderFrameObserver.
void DidCreatePepperPlugin(content::RendererPpapiHost* host) override;
void OnDestruct() override;

private:
DISALLOW_COPY_AND_ASSIGN(PepperHelper);
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/plugins/plugin_preroller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,7 @@ void PluginPreroller::OnThrottlerDestroyed() {
throttler_ = nullptr;
delete this;
}

void PluginPreroller::OnDestruct() {
delete this;
}
3 changes: 3 additions & 0 deletions chrome/renderer/plugins/plugin_preroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ class PluginPreroller : public content::PluginInstanceThrottler::Observer,
void OnThrottleStateChange() override;
void OnThrottlerDestroyed() override;

// content::RenderFrameObserver implementation.
void OnDestruct() override;

blink::WebLocalFrame* frame_;
blink::WebPluginParams params_;
content::WebPluginInfo info_;
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/prerender/prerender_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,8 @@ void PrerenderHelper::OnSetIsPrerendering(bool is_prerendering) {
UpdateVisibilityState(frame);
}

void PrerenderHelper::OnDestruct() {
delete this;
}

} // namespace prerender
3 changes: 2 additions & 1 deletion chrome/renderer/prerender/prerender_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ class PrerenderHelper
static bool IsPrerendering(const content::RenderFrame* render_frame);

private:
// RenderViewObserver implementation
// RenderFrameObserver implementation.
bool OnMessageReceived(const IPC::Message& message) override;
void OnDestruct() override;

void OnSetIsPrerendering(bool is_prerendering);

Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/prerender/prerenderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,8 @@ void PrerendererClient::willAddPrerender(
render_view()->GetSize()));
}

void PrerendererClient::OnDestruct() {
delete this;
}

} // namespace prerender
3 changes: 3 additions & 0 deletions chrome/renderer/prerender/prerenderer_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ class PrerendererClient : public content::RenderViewObserver,
private:
~PrerendererClient() override;

// RenderViewObserver implementation.
void OnDestruct() override;

// Implements blink::WebPrerendererClient
void willAddPrerender(blink::WebPrerender* prerender) override;
};
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/safe_browsing/phishing_classifier_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,4 +289,8 @@ void PhishingClassifierDelegate::MaybeStartClassification() {
base::Unretained(this)));
}

void PhishingClassifierDelegate::OnDestruct() {
delete this;
}

} // namespace safe_browsing
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class PhishingClassifierDelegate : public content::RenderFrameObserver {

// RenderFrameObserver implementation.
bool OnMessageReceived(const IPC::Message& message) override;
void OnDestruct() override;

// Called by the RenderFrame when it receives a StartPhishingDetection IPC
// from the browser. This signals that it is ok to begin classification
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/safe_browsing/threat_dom_details.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,8 @@ void ThreatDOMDetails::ExtractResources(
resources->push_back(details_node);
}

void ThreatDOMDetails::OnDestruct() {
delete this;
}

} // namespace safe_browsing
1 change: 1 addition & 0 deletions chrome/renderer/safe_browsing/threat_dom_details.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class ThreatDOMDetails : public content::RenderFrameObserver {

// RenderFrameObserver implementation.
bool OnMessageReceived(const IPC::Message& message) override;
void OnDestruct() override;

void OnGetThreatDOMDetails();

Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/searchbox/searchbox.cc
Original file line number Diff line number Diff line change
Expand Up @@ -537,3 +537,7 @@ void SearchBox::Reset() {
is_key_capture_enabled_ = false;
theme_info_ = ThemeBackgroundInfo();
}

void SearchBox::OnDestruct() {
delete this;
}
1 change: 1 addition & 0 deletions chrome/renderer/searchbox/searchbox.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ class SearchBox : public content::RenderViewObserver,
private:
// Overridden from content::RenderViewObserver:
bool OnMessageReceived(const IPC::Message& message) override;
void OnDestruct() override;

void OnSetPageSequenceNumber(int page_seq_no);
void OnChromeIdentityCheckResult(const base::string16& identity,
Expand Down
4 changes: 4 additions & 0 deletions chrome/renderer/spellchecker/spellcheck_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -349,3 +349,7 @@ bool SpellCheckProvider::SatisfyRequestFromCache(

return false;
}

void SpellCheckProvider::OnDestruct() {
delete this;
}
3 changes: 3 additions & 0 deletions chrome/renderer/spellchecker/spellcheck_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class SpellCheckProvider
bool SatisfyRequestFromCache(const base::string16& text,
blink::WebTextCheckingCompletion* completion);

// RenderViewObserver implementation.
void OnDestruct() override;

// blink::WebSpellCheckClient implementation.
void spellCheck(
const blink::WebString& text,
Expand Down
4 changes: 4 additions & 0 deletions chromecast/renderer/cast_media_load_deferrer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,8 @@ void CastMediaLoadDeferrer::WasShown() {
delete this;
}

void CastMediaLoadDeferrer::OnDestruct() {
delete this;
}

} // namespace chromecast
1 change: 1 addition & 0 deletions chromecast/renderer/cast_media_load_deferrer.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class CastMediaLoadDeferrer : public content::RenderFrameObserver {
private:
// content::RenderFrameObserver implementation:
void WasShown() override;
void OnDestruct() override;

base::Closure continue_loading_cb_;

Expand Down
4 changes: 4 additions & 0 deletions components/autofill/content/renderer/page_click_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ void PageClickTracker::DoFocusChangeComplete() {
focused_node_was_last_clicked_ = false;
}

void PageClickTracker::OnDestruct() {
delete this;
}

// PageClickTracker::Legacy ----------------------------------------------------

PageClickTracker::Legacy::Legacy(PageClickTracker* tracker)
Expand Down
1 change: 1 addition & 0 deletions components/autofill/content/renderer/page_click_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class PageClickTracker : public content::RenderFrameObserver {

// RenderFrameObserver implementation.
void FocusedNodeChanged(const blink::WebNode& node) override;
void OnDestruct() override;

// RenderViewObserver methods forwarded from Legacy. Should be
// merged into RenderFrameObserver.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,8 @@ void OverlayJsRenderFrameObserver::DidFinishLoad() {
->RemoveService<mojom::OverlayPageNotifierService>();
}

void OverlayJsRenderFrameObserver::OnDestruct() {
delete this;
}

} // namespace contextual_search
Loading

0 comments on commit 694b50a

Please sign in to comment.