Skip to content

Commit

Permalink
Handle automation server crashes. When Chrome crashes, we now handle …
Browse files Browse the repository at this point in the history
…the case and support document refresh or reload.

When chrome crashes, we draw a poor man's sad tab (":-("), so that can clearly be improved.
Another thing is that if the chrome instance that crashed held several navigational entries, then that history is lost.

TEST=There are a couple of tests included, so run those (*TabCrash*) and also verify that when the chrome automation server is killed that we do the right thing.  Also check info in bug report.
BUG=25839

Review URL: http://codereview.chromium.org/3061036

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@55565 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
tommi@chromium.org committed Aug 10, 2010
1 parent e721ebe commit bbfa9a1
Show file tree
Hide file tree
Showing 22 changed files with 942 additions and 437 deletions.
4 changes: 3 additions & 1 deletion chrome/test/automation/automation_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ static const int kSleepTime = 250;
// Used by AutomationProxy, declared here so that other headers don't need
// to include automation_proxy.h.
enum AutomationLaunchResult {
AUTOMATION_LAUNCH_RESULT_INVALID = -1,
AUTOMATION_SUCCESS,
AUTOMATION_TIMEOUT,
AUTOMATION_VERSION_MISMATCH,
AUTOMATION_CREATE_TAB_FAILED
AUTOMATION_CREATE_TAB_FAILED,
AUTOMATION_SERVER_CRASHED,
};

enum AutomationMsg_NavigationResponseValues {
Expand Down
46 changes: 32 additions & 14 deletions chrome_frame/chrome_active_document.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ ChromeActiveDocument::ChromeActiveDocument()
accelerator_table_(NULL) {
TRACE_EVENT_BEGIN("chromeframe.createactivedocument", this, "");

url_fetcher_.set_frame_busting(false);
url_fetcher_->set_frame_busting(false);
memset(&navigation_info_, 0, sizeof(navigation_info_));
}

Expand All @@ -83,7 +83,7 @@ HRESULT ChromeActiveDocument::FinalConstruct() {
DLOG(INFO) << "Reusing automation client instance from "
<< cached_document;
DCHECK(automation_client_.get() != NULL);
automation_client_->Reinitialize(this, &url_fetcher_);
automation_client_->Reinitialize(this, url_fetcher_.get());
is_automation_client_reused_ = true;
} else {
// The FinalConstruct implementation in the ChromeFrameActivexBase class
Expand Down Expand Up @@ -277,7 +277,8 @@ STDMETHODIMP ChromeActiveDocument::Load(BOOL fully_avalable,
return E_INVALIDARG;
}

std::string referrer = mgr ? mgr->referrer() : EmptyString();
std::string referrer(mgr ? mgr->referrer() : EmptyString());

// With CTransaction patch we have more robust way to grab the referrer for
// each top-level-switch-to-CF request by peeking at our sniffing data
// object that lives inside the bind context.
Expand All @@ -293,7 +294,7 @@ STDMETHODIMP ChromeActiveDocument::Load(BOOL fully_avalable,
}

if (!cf_url.is_chrome_protocol() && !cf_url.attach_to_external_tab())
url_fetcher_.SetInfoForUrl(cf_url.url(), moniker_name, bind_context);
url_fetcher_->SetInfoForUrl(cf_url.url(), moniker_name, bind_context);

THREAD_SAFE_UMA_HISTOGRAM_CUSTOM_COUNTS("ChromeFrame.FullTabLaunchType",
cf_url.is_chrome_protocol(),
Expand Down Expand Up @@ -372,7 +373,12 @@ STDMETHODIMP ChromeActiveDocument::Exec(const GUID* cmd_group_guid,
if (automation_client_.get() && automation_client_->tab()) {
return ProcessExecCommand(cmd_group_guid, command_id, cmd_exec_opt,
in_args, out_args);
} else if (command_id == OLECMDID_REFRESH && cmd_group_guid == NULL) {
// If the automation server has crashed and the user is refreshing the
// page, let OnRefreshPage attempt to recover.
OnRefreshPage(cmd_group_guid, command_id, cmd_exec_opt, in_args, out_args);
}

return OLECMDERR_E_NOTSUPPORTED;
}

Expand Down Expand Up @@ -844,7 +850,7 @@ void ChromeActiveDocument::OnDetermineSecurityZone(const GUID* cmd_group_guid,
}

void ChromeActiveDocument::OnDisplayPrivacyInfo() {
privacy_info_ = url_fetcher_.privacy_info();
privacy_info_ = url_fetcher_->privacy_info();
Reset();
DoPrivacyDlg(m_hWnd, url_, this, TRUE);
}
Expand Down Expand Up @@ -989,10 +995,8 @@ bool ChromeActiveDocument::LaunchUrl(const ChromeFrameUrl& cf_url,

url_.Allocate(cf_url.url().c_str());

std::string utf8_url;
WideToUTF8(url_, url_.Length(), &utf8_url);

DLOG(INFO) << "this:" << this << " url is:" << url_;
std::string utf8_url(WideToUTF8(cf_url.url()));
DLOG(INFO) << "this:" << this << " url is:" << utf8_url;

if (cf_url.attach_to_external_tab()) {
dimensions_ = cf_url.dimensions();
Expand All @@ -1010,15 +1014,17 @@ bool ChromeActiveDocument::LaunchUrl(const ChromeFrameUrl& cf_url,
if (is_automation_client_reused_)
return true;

automation_client_->SetUrlFetcher(&url_fetcher_);
automation_client_->SetUrlFetcher(url_fetcher_.get());

GURL url(utf8_url);
return InitializeAutomation(GetHostProcessName(false), L"", IsIEInPrivate(),
false, GURL(utf8_url), GURL(referrer));
false, url, GURL(referrer));
}


HRESULT ChromeActiveDocument::OnRefreshPage(const GUID* cmd_group_guid,
DWORD command_id, DWORD cmd_exec_opt, VARIANT* in_args, VARIANT* out_args) {
DLOG(INFO) << __FUNCTION__;
popup_allowed_ = false;
if (in_args->vt == VT_I4 &&
in_args->lVal & OLECMDIDF_REFRESH_PAGEACTION_POPUPWINDOW) {
Expand All @@ -1027,17 +1033,29 @@ HRESULT ChromeActiveDocument::OnRefreshPage(const GUID* cmd_group_guid,
// Ask the yellow security band to change the text and icon and to remain
// visible.
IEExec(&CGID_DocHostCommandHandler, OLECMDID_PAGEACTIONBLOCKED,
0x80000000 | OLECMDIDF_WINDOWSTATE_USERVISIBLE_VALID, NULL, NULL);
0x80000000 | OLECMDIDF_WINDOWSTATE_USERVISIBLE_VALID, NULL, NULL);
}

TabProxy* tab_proxy = GetTabProxy();
if (tab_proxy)
if (tab_proxy) {
tab_proxy->ReloadAsync();
} else {
DLOG(ERROR) << "No automation proxy";
// The current url request manager (url_fetcher_) has been switched to
// a stopping state so we need to reset it and get a new one for the new
// automation server.
ResetUrlRequestManager();
url_fetcher_->set_frame_busting(false);
// And now launch the current URL again. This starts a new server process.
DCHECK(navigation_info_.url.is_valid());
ChromeFrameUrl cf_url;
cf_url.Parse(UTF8ToWide(navigation_info_.url.spec()));
LaunchUrl(cf_url, navigation_info_.referrer.spec());
}

return S_OK;
}


HRESULT ChromeActiveDocument::SetPageFontSize(const GUID* cmd_group_guid,
DWORD command_id,
DWORD cmd_exec_opt,
Expand Down
7 changes: 2 additions & 5 deletions chrome_frame/chrome_active_document.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
#include <atlcom.h>
#include <atlctl.h>
#include <htiframe.h>
#include <map>
#include <mshtmcid.h>
#include <perhist.h>

#include <map>
#include <string>

#include "base/scoped_ptr.h"
Expand Down Expand Up @@ -428,10 +429,6 @@ END_EXEC_COMMAND_MAP()
LRESULT OnSetFocus(UINT message, WPARAM wparam, LPARAM lparam,
BOOL& handled);

// Checks for the presence of known-to-be-buggy BHOs. If we find any
// we do not fire the DocumentComplete event to avoid a crash.
static bool ShouldFireDocumentComplete();

// Sets the dimensions on the IE window. These dimensions are parsed out from
// the information passed in from Chrome during window.open.
void SetWindowDimensions();
Expand Down
6 changes: 3 additions & 3 deletions chrome_frame/chrome_frame_activex.cc
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ HRESULT ChromeFrameActivex::IOleObject_SetClientSite(
if (SUCCEEDED(service_hr) && wants_privileged)
is_privileged_ = true;

url_fetcher_.set_privileged_mode(is_privileged_);
url_fetcher_->set_privileged_mode(is_privileged_);
}

std::wstring chrome_extra_arguments;
Expand Down Expand Up @@ -458,8 +458,8 @@ HRESULT ChromeFrameActivex::IOleObject_SetClientSite(
WideToUTF8(url_, url_.Length(), &utf8_url);
}

url_fetcher_.set_frame_busting(!is_privileged_);
automation_client_->SetUrlFetcher(&url_fetcher_);
url_fetcher_->set_frame_busting(!is_privileged_);
automation_client_->SetUrlFetcher(url_fetcher_.get());
if (!InitializeAutomation(profile_name, chrome_extra_arguments,
IsIEInPrivate(), true, GURL(utf8_url),
GURL())) {
Expand Down
1 change: 0 additions & 1 deletion chrome_frame/chrome_frame_activex.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ BEGIN_COM_MAP(ChromeFrameActivex)
COM_INTERFACE_ENTRY(IObjectWithSite)
COM_INTERFACE_ENTRY(IObjectSafety)
COM_INTERFACE_ENTRY(IPersistPropertyBag)
COM_INTERFACE_ENTRY(IConnectionPointContainer)
COM_INTERFACE_ENTRY_CHAIN(Base)
END_COM_MAP()

Expand Down
42 changes: 36 additions & 6 deletions chrome_frame/chrome_frame_activex_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,15 @@ class ATL_NO_VTABLE ChromeFrameActivexBase : // NOLINT
public:
ChromeFrameActivexBase()
: ready_state_(READYSTATE_UNINITIALIZED),
failed_to_fetch_in_place_frame_(false) {
url_fetcher_(new UrlmonUrlRequestManager()),
failed_to_fetch_in_place_frame_(false),
draw_sad_tab_(false) {
m_bWindowOnly = TRUE;
url_fetcher_.set_container(static_cast<IDispatch*>(this));
url_fetcher_->set_container(static_cast<IDispatch*>(this));
}

~ChromeFrameActivexBase() {
url_fetcher_.set_container(NULL);
url_fetcher_->set_container(NULL);
}

DECLARE_OLEMISC_STATUS(OLEMISC_RECOMPOSEONRESIZE | OLEMISC_CANTLINKINSIDE |
Expand All @@ -206,6 +208,7 @@ BEGIN_COM_MAP(ChromeFrameActivexBase)
COM_INTERFACE_ENTRY(IQuickActivate)
COM_INTERFACE_ENTRY(IProvideClassInfo)
COM_INTERFACE_ENTRY(IProvideClassInfo2)
COM_INTERFACE_ENTRY(IConnectionPointContainer)
COM_INTERFACE_ENTRY_FUNC_BLIND(0, InterfaceNotSupported)
END_COM_MAP()

Expand Down Expand Up @@ -270,6 +273,10 @@ END_MSG_MAP()
Uninitialize();
}

void ResetUrlRequestManager() {
url_fetcher_.reset(new UrlmonUrlRequestManager());
}

static HRESULT WINAPI InterfaceNotSupported(void* pv, REFIID riid, void** ppv,
DWORD dw) {
#ifndef NDEBUG
Expand Down Expand Up @@ -300,7 +307,23 @@ END_MSG_MAP()
NOTREACHED();
return E_FAIL;
}
// Don't draw anything.

if (draw_sad_tab_) {
// TODO(tommi): Draw a proper sad tab.
RECT rc = {0};
if (draw_info.prcBounds) {
rc.top = draw_info.prcBounds->top;
rc.bottom = draw_info.prcBounds->bottom;
rc.left = draw_info.prcBounds->left;
rc.right = draw_info.prcBounds->right;
} else {
GetClientRect(&rc);
}
::DrawTextA(draw_info.hdcDraw, ":-(", -1, &rc,
DT_CENTER | DT_VCENTER | DT_SINGLELINE);
} else {
// Don't draw anything.
}
return S_OK;
}

Expand Down Expand Up @@ -512,7 +535,7 @@ END_MSG_MAP()
LRESULT OnCreate(UINT message, WPARAM wparam, LPARAM lparam,
BOOL& handled) { // NO_LINT
ModifyStyle(0, WS_CLIPCHILDREN | WS_CLIPSIBLINGS, 0);
url_fetcher_.put_notification_window(m_hWnd);
url_fetcher_->put_notification_window(m_hWnd);
if (automation_client_.get()) {
automation_client_->SetParentWindow(m_hWnd);
} else {
Expand All @@ -535,6 +558,7 @@ END_MSG_MAP()

// ChromeFrameDelegate override
virtual void OnAutomationServerReady() {
draw_sad_tab_ = false;
ChromeFramePlugin<T>::OnAutomationServerReady();

ready_state_ = READYSTATE_COMPLETE;
Expand All @@ -544,6 +568,10 @@ END_MSG_MAP()
// ChromeFrameDelegate override
virtual void OnAutomationServerLaunchFailed(
AutomationLaunchResult reason, const std::string& server_version) {
DLOG(INFO) << __FUNCTION__;
if (reason == AUTOMATION_SERVER_CRASHED)
draw_sad_tab_ = true;

ready_state_ = READYSTATE_UNINITIALIZED;
FireOnChanged(DISPID_READYSTATE);
}
Expand Down Expand Up @@ -1201,6 +1229,8 @@ END_MSG_MAP()
// If false, we tried but failed to fetch an IOleInPlaceFrame from our host.
// Cached here so we don't try to fetch it every time if we keep failing.
bool failed_to_fetch_in_place_frame_;
bool draw_sad_tab_;

ScopedComPtr<IOleInPlaceFrame> in_place_frame_;

// For more information on the ready_state_ property see:
Expand All @@ -1222,7 +1252,7 @@ END_MSG_MAP()

// Handle network requests when host network stack is used. Passed to the
// automation client on initialization.
UrlmonUrlRequestManager url_fetcher_;
scoped_ptr<UrlmonUrlRequestManager> url_fetcher_;
};

#endif // CHROME_FRAME_CHROME_FRAME_ACTIVEX_BASE_H_
Loading

0 comments on commit bbfa9a1

Please sign in to comment.