From 79134a77f2230028465ffbc2b46813bf86a7d077 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Tue, 31 Aug 2010 14:10:31 +0000 Subject: [PATCH] Check for a valid pointer in all places where GetWebView() and GetWebViewHost() are directly used in order to prevent potential crashes after WM_DESTROY is processed (issue #84). git-svn-id: https://chromiumembedded.googlecode.com/svn/trunk@97 5089003a-bbd8-11dd-ad1f-f1f9622dbc98 --- libcef/browser_impl.cc | 49 ++++++++++++++++++-------- libcef/browser_impl_win.cc | 6 +++- libcef/browser_webview_delegate.cc | 22 ++++++++---- libcef/browser_webview_delegate_win.cc | 3 +- 4 files changed, 57 insertions(+), 23 deletions(-) diff --git a/libcef/browser_impl.cc b/libcef/browser_impl.cc index ee10f8ded..34609f664 100644 --- a/libcef/browser_impl.cc +++ b/libcef/browser_impl.cc @@ -164,17 +164,21 @@ CefRefPtr CefBrowserImpl::GetHandler() CefRefPtr CefBrowserImpl::GetMainFrame() { - return GetCefFrame(GetWebView()->mainFrame()); + return GetWebView() ? GetCefFrame(GetWebView()->mainFrame()) : NULL; } CefRefPtr CefBrowserImpl::GetFocusedFrame() { - return GetCefFrame(GetWebView()->focusedFrame()); + return GetWebView() ? GetCefFrame(GetWebView()->focusedFrame()) : NULL; } CefRefPtr CefBrowserImpl::GetFrame(const std::wstring& name) { - WebFrame* frame = GetWebView()->findFrameByName(name); + WebView* view = GetWebView(); + if (!view) + return NULL; + + WebFrame* frame = view->findFrameByName(name); if(frame) return GetCefFrame(frame); return NULL; @@ -183,6 +187,9 @@ CefRefPtr CefBrowserImpl::GetFrame(const std::wstring& name) void CefBrowserImpl::GetFrameNames(std::vector& names) { WebView* view = GetWebView(); + if (!view) + return; + WebFrame* main_frame = view->mainFrame(); WebFrame* it = main_frame; do { @@ -259,10 +266,14 @@ void CefBrowserImpl::RemoveCefFrame(const std::wstring& name) WebFrame* CefBrowserImpl::GetWebFrame(CefRefPtr frame) { + WebView* view = GetWebView(); + if (!view) + return NULL; + std::wstring name = frame->GetName(); if(name.empty()) - return GetWebView()->mainFrame(); - return GetWebView()->findFrameByName(name); + return view ->mainFrame(); + return view ->findFrameByName(name); } void CefBrowserImpl::Undo(CefRefPtr frame) @@ -663,12 +674,16 @@ bool CefBrowserImpl::UIT_Navigate(const BrowserNavigationEntry& entry, { REQUIRE_UIT(); + WebView* view = GetWebView(); + if (!view) + return false; + // Get the right target frame for the entry. WebFrame* frame; if (!entry.GetTargetFrame().empty()) - frame = GetWebView()->findFrameByName(entry.GetTargetFrame()); + frame = view->findFrameByName(entry.GetTargetFrame()); else - frame = GetWebView()->mainFrame(); + frame = view->mainFrame(); // TODO(mpcomplete): should we clear the target frame, or should // back/forward navigations maintain the target frame? @@ -731,7 +746,7 @@ bool CefBrowserImpl::UIT_Navigate(const BrowserNavigationEntry& entry, // setFocusedFrame() may be unnecessary or in the wrong place. See this // thread for additional details: // http://groups.google.com/group/chromium-dev/browse_thread/thread/42bcd31b59e3a168 - GetWebView()->setFocusedFrame(frame); + view->setFocusedFrame(frame); UIT_SetFocus(GetWebViewHost(), true); } @@ -795,7 +810,8 @@ void CefBrowserImpl::UIT_HandleAction(CefHandler::MenuId menuId, UIT_Reload(); break; case MENU_ID_NAV_STOP: - GetWebView()->mainFrame()->stopLoading(); + if (GetWebView()) + GetWebView()->mainFrame()->stopLoading(); break; case MENU_ID_UNDO: if(web_frame) @@ -907,7 +923,11 @@ void CefBrowserImpl::UIT_CanGoForwardNotify(bool *retVal, void CefBrowserImpl::UIT_Find(int identifier, const std::wstring& search_text, const WebKit::WebFindOptions& options) { - WebFrame* main_frame = GetWebView()->mainFrame(); + WebView* view = GetWebView(); + if (!view) + return; + + WebFrame* main_frame = view->mainFrame(); if (main_frame->document().isPluginDocument()) { WebPlugin* plugin = main_frame->document().to().plugin(); @@ -928,7 +948,7 @@ void CefBrowserImpl::UIT_Find(int identifier, const std::wstring& search_text, } WebFrame* frame_after_main = main_frame->traverseNext(true); - WebFrame* focused_frame = GetWebView()->focusedFrame(); + WebFrame* focused_frame = view->focusedFrame(); WebFrame* search_frame = focused_frame; // start searching focused frame. bool multi_frame = (frame_after_main != main_frame); @@ -977,7 +997,7 @@ void CefBrowserImpl::UIT_Find(int identifier, const std::wstring& search_text, } } - GetWebView()->setFocusedFrame(search_frame); + view->setFocusedFrame(search_frame); } while (!result && search_frame != focused_frame); if (options.findNext && current_selection.isNull()) { @@ -1070,6 +1090,7 @@ void CefBrowserImpl::UIT_NotifyFindStatus(int identifier, int count, bool CefFrameImpl::IsFocused() { - return (browser_->GetWebFrame(this) == - browser_->GetWebView()->focusedFrame()); + return (browser_->GetWebView() && + (browser_->GetWebFrame(this) == + browser_->GetWebView()->focusedFrame())); } diff --git a/libcef/browser_impl_win.cc b/libcef/browser_impl_win.cc index e8e8c6a99..86800e21a 100644 --- a/libcef/browser_impl_win.cc +++ b/libcef/browser_impl_win.cc @@ -46,6 +46,7 @@ LRESULT CALLBACK CefBrowserImpl::WndProc(HWND hwnd, UINT message, break; case WM_DESTROY: + if (browser) { CefRefPtr handler = browser->GetHandler(); if(handler.get()) { @@ -56,6 +57,7 @@ LRESULT CALLBACK CefBrowserImpl::WndProc(HWND hwnd, UINT message, // Clean up anything associated with the WebViewHost widget. browser->GetWebViewHost()->webwidget()->close(); + browser->webviewhost_.reset(); // Clear the user data pointer. win_util::SetWindowUserData(hwnd, NULL); @@ -150,7 +152,9 @@ void CefBrowserImpl::UIT_CreateBrowser(const std::wstring& url) void CefBrowserImpl::UIT_SetFocus(WebWidgetHost* host, bool enable) { REQUIRE_UIT(); - + if (!host) + return; + if (enable) ::SetFocus(host->view_handle()); else if (::GetFocus() == host->view_handle()) diff --git a/libcef/browser_webview_delegate.cc b/libcef/browser_webview_delegate.cc index 0768739f6..207d1cfed 100644 --- a/libcef/browser_webview_delegate.cc +++ b/libcef/browser_webview_delegate.cc @@ -129,14 +129,16 @@ int next_page_id_ = 1; void BrowserWebViewDelegate::SetUserStyleSheetEnabled(bool is_enabled) { WebPreferences* prefs = _Context->web_preferences(); prefs->user_style_sheet_enabled = is_enabled; - prefs->Apply(browser_->GetWebView()); + if (browser_->GetWebView()) + prefs->Apply(browser_->GetWebView()); } void BrowserWebViewDelegate::SetUserStyleSheetLocation(const GURL& location) { WebPreferences* prefs = _Context->web_preferences(); prefs->user_style_sheet_enabled = true; prefs->user_style_sheet_location = location; - prefs->Apply(browser_->GetWebView()); + if (browser_->GetWebView()) + prefs->Apply(browser_->GetWebView()); } // WebViewClient ------------------------------------------------------------- @@ -187,9 +189,10 @@ void BrowserWebViewDelegate::didAddMessageToConsole( } void BrowserWebViewDelegate::printPage(WebFrame* frame) { - if(frame == NULL) - frame = browser_->GetWebView()->mainFrame(); - browser_->UIT_PrintPages(frame); + if (!frame) + frame = browser_->GetWebView() ? browser_->GetWebView()->mainFrame() : NULL; + if (frame) + browser_->UIT_PrintPages(frame); } void BrowserWebViewDelegate::didStartLoading() { @@ -297,7 +300,8 @@ bool BrowserWebViewDelegate::handleCurrentKeyboardEvent() { if (edit_command_name_.empty()) return false; - WebFrame* frame = browser_->GetWebView()->focusedFrame(); + WebFrame* frame = browser_->GetWebView() ? + browser_->GetWebView()->focusedFrame() : NULL; if (!frame) return false; @@ -420,7 +424,8 @@ void BrowserWebViewDelegate::startDragging( //HRESULT res = DoDragDrop(drop_data.data_object, drag_delegate_.get(), // ok_effect, &effect); //DCHECK(DRAGDROP_S_DROP == res || DRAGDROP_S_CANCEL == res); - browser_->GetWebView()->dragSourceSystemDragEnded(); + if (browser_->GetWebView()) + browser_->GetWebView()->dragSourceSystemDragEnded(); } void BrowserWebViewDelegate::focusNext() { @@ -916,6 +921,9 @@ void BrowserWebViewDelegate::UpdateSessionHistory(WebFrame* frame) { if (!entry) return; + if (!browser_->GetWebView()) + return; + const WebHistoryItem& history_item = browser_->GetWebView()->mainFrame()->previousHistoryItem(); if (history_item.isNull()) diff --git a/libcef/browser_webview_delegate_win.cc b/libcef/browser_webview_delegate_win.cc index 721578250..89a4b3ff0 100644 --- a/libcef/browser_webview_delegate_win.cc +++ b/libcef/browser_webview_delegate_win.cc @@ -152,7 +152,8 @@ webkit_glue::WebPluginDelegate* BrowserWebViewDelegate::CreatePluginDelegate( const FilePath& file_path, const std::string& mime_type) { - HWND hwnd = browser_->GetWebViewHost()->view_handle(); + HWND hwnd = browser_->GetWebViewHost() ? + browser_->GetWebViewHost()->view_handle() : NULL; if (!hwnd) return NULL;