diff options
22 files changed, 942 insertions, 437 deletions
diff --git a/chrome/test/automation/automation_constants.h b/chrome/test/automation/automation_constants.h index 663c9bd..f34c0f2 100644 --- a/chrome/test/automation/automation_constants.h +++ b/chrome/test/automation/automation_constants.h @@ -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 { diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc index ee29183..1c03c94 100644 --- a/chrome_frame/chrome_active_document.cc +++ b/chrome_frame/chrome_active_document.cc @@ -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_)); } @@ -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 @@ -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. @@ -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(), @@ -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; } @@ -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); } @@ -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(); @@ -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) { @@ -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, diff --git a/chrome_frame/chrome_active_document.h b/chrome_frame/chrome_active_document.h index b8c59eb..7f9fd9d 100644 --- a/chrome_frame/chrome_active_document.h +++ b/chrome_frame/chrome_active_document.h @@ -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" @@ -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(); diff --git a/chrome_frame/chrome_frame_activex.cc b/chrome_frame/chrome_frame_activex.cc index eeffb23..27c415c 100644 --- a/chrome_frame/chrome_frame_activex.cc +++ b/chrome_frame/chrome_frame_activex.cc @@ -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; @@ -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())) { diff --git a/chrome_frame/chrome_frame_activex.h b/chrome_frame/chrome_frame_activex.h index ca77147..42c162b 100644 --- a/chrome_frame/chrome_frame_activex.h +++ b/chrome_frame/chrome_frame_activex.h @@ -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() diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h index c5d17d4..a2774e3 100644 --- a/chrome_frame/chrome_frame_activex_base.h +++ b/chrome_frame/chrome_frame_activex_base.h @@ -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 | @@ -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() @@ -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 @@ -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; } @@ -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 { @@ -535,6 +558,7 @@ END_MSG_MAP() // ChromeFrameDelegate override virtual void OnAutomationServerReady() { + draw_sad_tab_ = false; ChromeFramePlugin<T>::OnAutomationServerReady(); ready_state_ = READYSTATE_COMPLETE; @@ -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); } @@ -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: @@ -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_ diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc index ea2c127..32c4749 100644 --- a/chrome_frame/chrome_frame_automation.cc +++ b/chrome_frame/chrome_frame_automation.cc @@ -126,12 +126,13 @@ class ChromeFrameAutomationProxyImpl::CFMsgDispatcher }; ChromeFrameAutomationProxyImpl::ChromeFrameAutomationProxyImpl( - int launch_timeout) - : AutomationProxy(launch_timeout) { + AutomationProxyCacheEntry* entry, int launch_timeout) + : AutomationProxy(launch_timeout), proxy_entry_(entry) { TRACE_EVENT_BEGIN("chromeframe.automationproxy", this, ""); sync_ = new CFMsgDispatcher(); message_filter_ = new TabProxyNotificationMessageFilter(tracker_.get()); + // Order of filters is not important. channel_->AddFilter(message_filter_.get()); channel_->AddFilter(sync_.get()); @@ -152,6 +153,15 @@ void ChromeFrameAutomationProxyImpl::CancelAsync(void* key) { sync_->Cancel(key); } +void ChromeFrameAutomationProxyImpl::OnChannelError() { + DLOG(ERROR) << "Automation server died"; + if (proxy_entry_) { + proxy_entry_->OnChannelError(); + } else { + NOTREACHED(); + } +} + scoped_refptr<TabProxy> ChromeFrameAutomationProxyImpl::CreateTabProxy( int handle) { DCHECK(tracker_->GetResource(handle) == NULL); @@ -188,83 +198,41 @@ struct LaunchTimeStats { #endif }; -ProxyFactory::ProxyCacheEntry::ProxyCacheEntry(const std::wstring& profile) - : proxy(NULL), profile_name(profile), ref_count(1), - launch_result(AutomationLaunchResult(-1)) { - thread.reset(new base::Thread(WideToASCII(profile_name).c_str())); - thread->Start(); +DISABLE_RUNNABLE_METHOD_REFCOUNT(AutomationProxyCacheEntry); + +AutomationProxyCacheEntry::AutomationProxyCacheEntry( + ChromeFrameLaunchParams* params, LaunchDelegate* delegate) + : profile_name(params->profile_name()), + launch_result_(AUTOMATION_LAUNCH_RESULT_INVALID), + snapshots_(NULL), uma_send_interval_(1) { + DCHECK(delegate); + thread_.reset(new base::Thread(WideToASCII(profile_name).c_str())); + thread_->Start(); + thread_->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(this, + &AutomationProxyCacheEntry::CreateProxy, params, delegate)); } -DISABLE_RUNNABLE_METHOD_REFCOUNT(ProxyFactory); - -ProxyFactory::ProxyFactory() - : uma_send_interval_(0) { - uma_send_interval_ = GetConfigInt(kDefaultSendUMADataInterval, - kUmaSendIntervalValue); +AutomationProxyCacheEntry::~AutomationProxyCacheEntry() { + DLOG(INFO) << __FUNCTION__ << profile_name; } -ProxyFactory::~ProxyFactory() { - for (size_t i = 0; i < proxies_.container().size(); ++i) { - DWORD result = WaitForSingleObject(proxies_[i]->thread->thread_handle(), 0); - if (WAIT_OBJECT_0 != result) - // TODO(stoyan): Don't leak proxies on exit. - DLOG(ERROR) << "Proxies leaked on exit."; - } +void AutomationProxyCacheEntry::StartSendUmaInterval( + ChromeFrameHistogramSnapshots* snapshots, int send_interval) { + DCHECK(snapshots); + DCHECK(!snapshots_); + snapshots_ = snapshots; + uma_send_interval_ = send_interval; + thread_->message_loop()->PostDelayedTask(FROM_HERE, + NewRunnableMethod(this, &AutomationProxyCacheEntry::SendUMAData), + send_interval); } -void ProxyFactory::GetAutomationServer( - LaunchDelegate* delegate, const ChromeFrameLaunchParams& params, - void** automation_server_id) { - TRACE_EVENT_BEGIN("chromeframe.createproxy", this, ""); - - ProxyCacheEntry* entry = NULL; - // Find already existing launcher thread for given profile - AutoLock lock(lock_); - for (size_t i = 0; i < proxies_.container().size(); ++i) { - if (!lstrcmpiW(proxies_[i]->profile_name.c_str(), - params.profile_name.c_str())) { - entry = proxies_[i]; - DCHECK(entry->thread.get() != NULL); - break; - } - } - - if (entry == NULL) { - entry = new ProxyCacheEntry(params.profile_name); - proxies_.container().push_back(entry); - } else { - entry->ref_count++; - } - - DCHECK(delegate != NULL); - DCHECK(automation_server_id != NULL); - - *automation_server_id = entry; - // Note we always queue request to the launch thread, even if we already - // have established proxy object. A simple lock around entry->proxy = proxy - // would allow calling LaunchDelegate directly from here if - // entry->proxy != NULL. Drawback is that callback may be invoked either in - // main thread or in background thread, which may confuse the client. - entry->thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(this, - &ProxyFactory::CreateProxy, entry, params, delegate)); - - // IE uses the chrome frame provided UMA data uploading scheme. NPAPI - // continues to use Chrome to upload UMA data. - if (!CrashMetricsReporter::GetInstance()->active()) { - entry->thread->message_loop()->PostDelayedTask(FROM_HERE, - NewRunnableMethod(this, &ProxyFactory::SendUMAData, entry), - uma_send_interval_); - } -} - -void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry, - const ChromeFrameLaunchParams& params, - LaunchDelegate* delegate) { - DCHECK(entry->thread->thread_id() == PlatformThread::CurrentId()); - if (entry->proxy) { - delegate->LaunchComplete(entry->proxy, entry->launch_result); - return; - } +void AutomationProxyCacheEntry::CreateProxy(ChromeFrameLaunchParams* params, + LaunchDelegate* delegate) { + DCHECK(IsSameThread(PlatformThread::CurrentId())); + DCHECK(delegate); + DCHECK(params); + DCHECK(proxy_.get() == NULL); // We *must* create automationproxy in a thread that has message loop, // since SyncChannel::Context construction registers event to be watched @@ -273,8 +241,7 @@ void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry, // At same time we must destroy/stop the thread from another thread. ChromeFrameAutomationProxyImpl* proxy = - new ChromeFrameAutomationProxyImpl( - params.automation_server_launch_timeout); + new ChromeFrameAutomationProxyImpl(this, params->launch_timeout()); // Launch browser scoped_ptr<CommandLine> command_line( @@ -303,20 +270,21 @@ void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry, if (IsHeadlessMode()) command_line->AppendSwitch(switches::kFullMemoryCrashReport); - DLOG(INFO) << "Profile path: " << params.profile_path.value(); - command_line->AppendSwitchPath(switches::kUserDataDir, params.profile_path); + DLOG(INFO) << "Profile path: " << params->profile_path().value(); + command_line->AppendSwitchPath(switches::kUserDataDir, + params->profile_path()); std::wstring command_line_string(command_line->command_line_string()); // If there are any extra arguments, append them to the command line. - if (!params.extra_chrome_arguments.empty()) { - command_line_string += L' ' + params.extra_chrome_arguments; + if (!params->extra_arguments().empty()) { + command_line_string += L' ' + params->extra_arguments(); } automation_server_launch_start_time_ = base::TimeTicks::Now(); if (!base::LaunchApp(command_line_string, false, false, NULL)) { // We have no code for launch failure. - entry->launch_result = AutomationLaunchResult(-1); + launch_result_ = AUTOMATION_LAUNCH_RESULT_INVALID; } else { // Launch timeout may happen if the new instance tries to communicate // with an existing Chrome instance that is hung and displays msgbox @@ -332,13 +300,13 @@ void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry, LaunchTimeStats launch_stats; // Wait for the automation server launch result, then stash away the // version string it reported. - entry->launch_result = proxy->WaitForAppLaunch(); + launch_result_ = proxy->WaitForAppLaunch(); launch_stats.Dump(); base::TimeDelta delta = base::TimeTicks::Now() - automation_server_launch_start_time_; - if (entry->launch_result == AUTOMATION_SUCCESS) { + if (launch_result_ == AUTOMATION_SUCCESS) { THREAD_SAFE_UMA_HISTOGRAM_TIMES( "ChromeFrame.AutomationServerLaunchSuccessTime", delta); } else { @@ -347,7 +315,7 @@ void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry, } THREAD_SAFE_UMA_HISTOGRAM_CUSTOM_COUNTS("ChromeFrame.LaunchResult", - entry->launch_result, + launch_result_, AUTOMATION_SUCCESS, AUTOMATION_CREATE_TAB_FAILED, AUTOMATION_CREATE_TAB_FAILED + 1); @@ -356,101 +324,201 @@ void ProxyFactory::CreateProxy(ProxyFactory::ProxyCacheEntry* entry, TRACE_EVENT_END("chromeframe.createproxy", this, ""); // Finally set the proxy. - entry->proxy = proxy; - delegate->LaunchComplete(proxy, entry->launch_result); + proxy_.reset(proxy); + launch_delegates_.push_back(delegate); + + delegate->LaunchComplete(proxy_.get(), launch_result_); } -bool ProxyFactory::ReleaseAutomationServer(void* server_id) { - if (!server_id) { - NOTREACHED(); - return false; - } +void AutomationProxyCacheEntry::RemoveDelegate(LaunchDelegate* delegate, + base::WaitableEvent* done, + bool* was_last_delegate) { + DCHECK(IsSameThread(PlatformThread::CurrentId())); + DCHECK(delegate); + DCHECK(done); + DCHECK(was_last_delegate); - ProxyCacheEntry* entry = reinterpret_cast<ProxyCacheEntry*>(server_id); + *was_last_delegate = false; -#ifndef NDEBUG - lock_.Acquire(); - Vector::ContainerType::iterator it = std::find(proxies_.container().begin(), - proxies_.container().end(), - entry); - DCHECK(it != proxies_.container().end()); - DCHECK(entry->thread->thread_id() != PlatformThread::CurrentId()); - DCHECK_GT(entry->ref_count, 0); + LaunchDelegates::iterator it = std::find(launch_delegates_.begin(), + launch_delegates_.end(), delegate); + if (it == launch_delegates_.end()) { + NOTREACHED(); + } else { + if (launch_delegates_.size() == 1) { + *was_last_delegate = true; - lock_.Release(); -#endif + if (snapshots_) + SendUMAData(); - base::WaitableEvent done(true, false); - entry->thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(this, - &ProxyFactory::ReleaseProxy, entry, &done)); - done.Wait(); + // Take down the proxy since we no longer have any clients. + proxy_.reset(NULL); - // Stop the thread and destroy the entry if there is no more clients. - if (entry->ref_count == 0) { - DCHECK(entry->proxy == NULL); - entry->thread.reset(); - delete entry; + // Process pending notifications. + thread_->message_loop()->RunAllPending(); + } + // Be careful to remove from the list after running pending + // tasks. Otherwise the delegate being removed might miss out + // on pending notifications such as LaunchComplete. + launch_delegates_.erase(it); } - return true; + done->Signal(); } -void ProxyFactory::ReleaseProxy(ProxyCacheEntry* entry, - base::WaitableEvent* done) { - DCHECK(entry->thread->thread_id() == PlatformThread::CurrentId()); +void AutomationProxyCacheEntry::AddDelegate(LaunchDelegate* delegate) { + DCHECK(IsSameThread(PlatformThread::CurrentId())); + DCHECK(std::find(launch_delegates_.begin(), + launch_delegates_.end(), + delegate) == launch_delegates_.end()) + << "Same delegate being added twice"; + DCHECK(launch_result_ != AUTOMATION_LAUNCH_RESULT_INVALID); - lock_.Acquire(); - if (!--entry->ref_count) { - Vector::ContainerType::iterator it = std::find(proxies_.container().begin(), - proxies_.container().end(), - entry); - proxies_->erase(it); - } - lock_.Release(); + launch_delegates_.push_back(delegate); + delegate->LaunchComplete(proxy_.get(), launch_result_); +} - // Send pending UMA data if any. - if (!entry->ref_count) { - SendUMAData(entry); - delete entry->proxy; - entry->proxy = NULL; +void AutomationProxyCacheEntry::OnChannelError() { + DCHECK(IsSameThread(PlatformThread::CurrentId())); + launch_result_ = AUTOMATION_SERVER_CRASHED; + LaunchDelegates::const_iterator it = launch_delegates_.begin(); + for (; it != launch_delegates_.end(); ++it) { + (*it)->AutomationServerDied(); } - - done->Signal(); } -Singleton<ProxyFactory> g_proxy_factory; - -void ProxyFactory::SendUMAData(ProxyCacheEntry* proxy_entry) { +void AutomationProxyCacheEntry::SendUMAData() { + DCHECK(IsSameThread(PlatformThread::CurrentId())); + DCHECK(snapshots_); // IE uses the chrome frame provided UMA data uploading scheme. NPAPI // continues to use Chrome to upload UMA data. if (CrashMetricsReporter::GetInstance()->active()) { return; } - if (!proxy_entry) { + if (!proxy_.get()) { NOTREACHED() << __FUNCTION__ << " Invalid proxy entry"; - return; + } else { + ChromeFrameHistogramSnapshots::HistogramPickledList histograms = + snapshots_->GatherAllHistograms(); + + if (!histograms.empty()) { + proxy_->Send(new AutomationMsg_RecordHistograms(0, histograms)); + } + + MessageLoop::current()->PostDelayedTask(FROM_HERE, + NewRunnableMethod(this, &AutomationProxyCacheEntry::SendUMAData), + uma_send_interval_); } +} - DCHECK(proxy_entry->thread->thread_id() == PlatformThread::CurrentId()); - if (proxy_entry->proxy) { - ChromeFrameHistogramSnapshots::HistogramPickledList histograms = - chrome_frame_histograms_.GatherAllHistograms(); +DISABLE_RUNNABLE_METHOD_REFCOUNT(ProxyFactory); - if (!histograms.empty()) { - proxy_entry->proxy->Send( - new AutomationMsg_RecordHistograms(0, histograms)); +ProxyFactory::ProxyFactory() + : uma_send_interval_(0) { + uma_send_interval_ = GetConfigInt(kDefaultSendUMADataInterval, + kUmaSendIntervalValue); +} + +ProxyFactory::~ProxyFactory() { + for (size_t i = 0; i < proxies_.container().size(); ++i) { + DWORD result = proxies_[i]->WaitForThread(0); + if (WAIT_OBJECT_0 != result) + // TODO(stoyan): Don't leak proxies on exit. + DLOG(ERROR) << "Proxies leaked on exit."; + } +} + +void ProxyFactory::GetAutomationServer( + LaunchDelegate* delegate, ChromeFrameLaunchParams* params, + void** automation_server_id) { + TRACE_EVENT_BEGIN("chromeframe.createproxy", this, ""); + + scoped_refptr<AutomationProxyCacheEntry> entry; + // Find already existing launcher thread for given profile + AutoLock lock(lock_); + for (size_t i = 0; i < proxies_.container().size(); ++i) { + if (proxies_[i]->IsSameProfile(params->profile_name())) { + entry = proxies_[i]; + break; } - } else { - DLOG(INFO) << __FUNCTION__ << " No proxy available to service the request"; - return; } - MessageLoop::current()->PostDelayedTask(FROM_HERE, NewRunnableMethod( - this, &ProxyFactory::SendUMAData, proxy_entry), uma_send_interval_); + if (entry == NULL) { + DLOG(INFO) << __FUNCTION__ << " creating new proxy entry"; + entry = new AutomationProxyCacheEntry(params, delegate); + proxies_.container().push_back(entry); + + // IE uses the chrome frame provided UMA data uploading scheme. NPAPI + // continues to use Chrome to upload UMA data. + if (!CrashMetricsReporter::GetInstance()->active()) { + entry->StartSendUmaInterval(&chrome_frame_histograms_, + uma_send_interval_); + } + } else if (delegate) { + // Notify the new delegate of the launch status from the worker thread + // and add it to the list of delegates. + entry->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(entry.get(), + &AutomationProxyCacheEntry::AddDelegate, delegate)); + } + + DCHECK(automation_server_id != NULL); + DCHECK(!entry->IsSameThread(PlatformThread::CurrentId())); + + *automation_server_id = entry; +} + +bool ProxyFactory::ReleaseAutomationServer(void* server_id, + LaunchDelegate* delegate) { + if (!server_id) { + NOTREACHED(); + return false; + } + + AutomationProxyCacheEntry* entry = + reinterpret_cast<AutomationProxyCacheEntry*>(server_id); + +#ifndef NDEBUG + lock_.Acquire(); + Vector::ContainerType::iterator it = std::find(proxies_.container().begin(), + proxies_.container().end(), + entry); + DCHECK(it != proxies_.container().end()); + DCHECK(!entry->IsSameThread(PlatformThread::CurrentId())); + + lock_.Release(); +#endif + + // AddRef the entry object as we might need to take it out of the proxy + // stack and then uninitialize the entry. + entry->AddRef(); + + bool last_delegate = false; + if (delegate) { + base::WaitableEvent done(true, false); + entry->message_loop()->PostTask(FROM_HERE, NewRunnableMethod(entry, + &AutomationProxyCacheEntry::RemoveDelegate, delegate, &done, + &last_delegate)); + done.Wait(); + } + + if (last_delegate) { + lock_.Acquire(); + Vector::ContainerType::iterator it = std::find(proxies_.container().begin(), + proxies_.container().end(), + entry); + proxies_.container().erase(it); + lock_.Release(); + } + + entry->Release(); + + return true; } +Singleton<ProxyFactory> g_proxy_factory; + template <> struct RunnableMethodTraits<ChromeFrameAutomationClient> { static void RetainCallee(ChromeFrameAutomationClient* obj) {} static void ReleaseCallee(ChromeFrameAutomationClient* obj) {} @@ -482,10 +550,18 @@ ChromeFrameAutomationClient::~ChromeFrameAutomationClient() { bool ChromeFrameAutomationClient::Initialize( ChromeFrameDelegate* chrome_frame_delegate, - const ChromeFrameLaunchParams& chrome_launch_params) { + ChromeFrameLaunchParams* chrome_launch_params) { DCHECK(!IsWindow()); chrome_frame_delegate_ = chrome_frame_delegate; +#ifndef NDEBUG + if (chrome_launch_params_ && chrome_launch_params_ != chrome_launch_params) { + DCHECK_EQ(chrome_launch_params_->url(), chrome_launch_params->url()); + DCHECK_EQ(chrome_launch_params_->referrer(), + chrome_launch_params->referrer()); + } +#endif + chrome_launch_params_ = chrome_launch_params; ui_thread_id_ = PlatformThread::CurrentId(); @@ -495,11 +571,12 @@ bool ChromeFrameAutomationClient::Initialize( // Don't use INFINITE (which is -1) or even MAXINT since we will convert // from milliseconds to microseconds when stored in a base::TimeDelta, // thus * 1000. An hour should be enough. - chrome_launch_params_.automation_server_launch_timeout = 60 * 60 * 1000; + chrome_launch_params_->set_launch_timeout(60 * 60 * 1000); } else { - DCHECK_LT(chrome_launch_params_.automation_server_launch_timeout, + DCHECK_LT(chrome_launch_params_->launch_timeout(), MAXINT / 2000); - chrome_launch_params_.automation_server_launch_timeout *= 2; + chrome_launch_params_->set_launch_timeout( + chrome_launch_params_->launch_timeout() * 2); } #endif // NDEBUG @@ -515,18 +592,17 @@ bool ChromeFrameAutomationClient::Initialize( } // Keep object in memory, while the window is alive. - // Corresponsing Release is in OnFinalMessage(); + // Corresponding Release is in OnFinalMessage(); AddRef(); // Mark our state as initializing. We'll reach initialized once // InitializeComplete is called successfully. init_state_ = INITIALIZING; - if (chrome_launch_params_.url.is_valid()) + if (chrome_launch_params_->url().is_valid()) navigate_after_initialization_ = false; - proxy_factory_->GetAutomationServer( - static_cast<ProxyFactory::LaunchDelegate*>(this), + proxy_factory_->GetAutomationServer(static_cast<LaunchDelegate*>(this), chrome_launch_params_, &automation_server_id_); return true; @@ -571,12 +647,15 @@ void ChromeFrameAutomationClient::Uninitialize() { if (::IsWindow(m_hWnd)) DestroyWindow(); + DCHECK(navigate_after_initialization_ == false); + handle_top_level_requests_ = false; + ui_thread_id_ = 0; chrome_frame_delegate_ = NULL; init_state_ = UNINITIALIZED; } -bool ChromeFrameAutomationClient::InitiateNavigation( - const std::string& url, const std::string& referrer, bool is_privileged) { +bool ChromeFrameAutomationClient::InitiateNavigation(const std::string& url, + const std::string& referrer, bool is_privileged) { if (url.empty()) return false; @@ -589,19 +668,26 @@ bool ChromeFrameAutomationClient::InitiateNavigation( return false; } - if (parsed_url != chrome_launch_params_.url) { + if (!chrome_launch_params_ || parsed_url != chrome_launch_params_->url()) { // Important: Since we will be using the referrer_ variable from a // different thread, we need to force a new std::string buffer instance for // the referrer_ GURL variable. Otherwise we can run into strangeness when // the GURL is accessed and it could result in a bad URL that can cause the // referrer to be dropped or something worse. - chrome_launch_params_.referrer = GURL(referrer.c_str()); - chrome_launch_params_.url = parsed_url; + GURL referrer_gurl(referrer.c_str()); + if (!chrome_launch_params_) { + FilePath profile_path; + chrome_launch_params_ = new ChromeFrameLaunchParams(parsed_url, + referrer_gurl, profile_path, L"", L"", false, false); + } else { + chrome_launch_params_->set_referrer(referrer_gurl); + chrome_launch_params_->set_url(parsed_url); + } navigate_after_initialization_ = false; if (is_initialized()) { - BeginNavigate(chrome_launch_params_.url, chrome_launch_params_.referrer); + BeginNavigate(); } else { navigate_after_initialization_ = true; } @@ -644,13 +730,12 @@ bool ChromeFrameAutomationClient::SetProxySettings( return true; } -void ChromeFrameAutomationClient::BeginNavigate(const GURL& url, - const GURL& referrer) { +void ChromeFrameAutomationClient::BeginNavigate() { // Could be NULL if we failed to launch Chrome in LaunchAutomationServer() if (!automation_server_ || !tab_.get()) { DLOG(WARNING) << "BeginNavigate - can't navigate."; ReportNavigationError(AUTOMATION_MSG_NAVIGATION_ERROR, - chrome_launch_params_.url.spec()); + chrome_launch_params_->url().spec()); return; } @@ -662,8 +747,9 @@ void ChromeFrameAutomationClient::BeginNavigate(const GURL& url, } IPC::SyncMessage* msg = - new AutomationMsg_NavigateInExternalTab(0, tab_->handle(), url, - referrer, NULL); + new AutomationMsg_NavigateInExternalTab(0, tab_->handle(), + chrome_launch_params_->url(), chrome_launch_params_->referrer(), + NULL); automation_server_->SendAsAsync(msg, new BeginNavigateContext(this), this); RECT client_rect = {0}; @@ -677,7 +763,7 @@ void ChromeFrameAutomationClient::BeginNavigateCompleted( AutomationMsg_NavigationResponseValues result) { if (result == AUTOMATION_MSG_NAVIGATION_ERROR) ReportNavigationError(AUTOMATION_MSG_NAVIGATION_ERROR, - chrome_launch_params_.url.spec()); + chrome_launch_params_->url().spec()); } void ChromeFrameAutomationClient::FindInPage(const std::wstring& search_string, @@ -797,7 +883,7 @@ void ChromeFrameAutomationClient::CreateExternalTab() { DCHECK(IsWindow()); DCHECK(automation_server_ != NULL); - if (chrome_launch_params_.url.is_valid()) { + if (chrome_launch_params_->url().is_valid()) { navigate_after_initialization_ = false; } @@ -805,12 +891,12 @@ void ChromeFrameAutomationClient::CreateExternalTab() { m_hWnd, gfx::Rect(), WS_CHILD, - chrome_launch_params_.incognito_mode, + chrome_launch_params_->incognito(), !use_chrome_network_, handle_top_level_requests_, - chrome_launch_params_.url, - chrome_launch_params_.referrer, - !chrome_launch_params_.is_widget_mode // Infobars disabled in widget mode. + chrome_launch_params_->url(), + chrome_launch_params_->referrer(), + !chrome_launch_params_->widget_mode() // Infobars disabled in widget mode. }; THREAD_SAFE_UMA_HISTOGRAM_CUSTOM_COUNTS( @@ -904,6 +990,16 @@ void ChromeFrameAutomationClient::LaunchComplete( } } +void ChromeFrameAutomationClient::AutomationServerDied() { + // Make sure we notify our delegate. + PostTask(FROM_HERE, NewRunnableMethod(this, + &ChromeFrameAutomationClient::InitializeComplete, + AUTOMATION_SERVER_CRASHED)); + // Then uninitialize. + PostTask(FROM_HERE, NewRunnableMethod(this, + &ChromeFrameAutomationClient::Uninitialize)); +} + void ChromeFrameAutomationClient::InitializeComplete( AutomationLaunchResult result) { DCHECK_EQ(PlatformThread::CurrentId(), ui_thread_id_); @@ -922,8 +1018,8 @@ void ChromeFrameAutomationClient::InitializeComplete( // If host specified destination URL - navigate. Apparently we do not use // accelerator table. if (navigate_after_initialization_) { - BeginNavigate(chrome_launch_params_.url, - chrome_launch_params_.referrer); + navigate_after_initialization_ = false; + BeginNavigate(); } } @@ -1126,7 +1222,7 @@ void ChromeFrameAutomationClient::ReleaseAutomationServer() { automation_server_->CancelAsync(this); } - proxy_factory_->ReleaseAutomationServer(server_id); + proxy_factory_->ReleaseAutomationServer(server_id, this); automation_server_ = NULL; // automation_server_ must not have been set to non NULL. diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h index 60fdcd4..4a3fcc5 100644 --- a/chrome_frame/chrome_frame_automation.h +++ b/chrome_frame/chrome_frame_automation.h @@ -51,19 +51,27 @@ struct DECLSPEC_NOVTABLE ChromeFrameAutomationProxy { // NOLINT virtual ~ChromeFrameAutomationProxy() {} }; +// Forward declarations. +class ProxyFactory; + // We extend the AutomationProxy class to handle our custom // IPC messages -class ChromeFrameAutomationProxyImpl : public ChromeFrameAutomationProxy, - // We have to derive from automationproxy since we want access to some members - // (tracker_ & channel_) - simple aggregation wont work; - // .. and non-public inheritance is verboten. - public AutomationProxy { +class ChromeFrameAutomationProxyImpl + : public ChromeFrameAutomationProxy, + // We have to derive from automationproxy since we want access to some + // members (tracker_ & channel_) - simple aggregation wont work; + // .. and non-public inheritance is verboten. + public AutomationProxy { public: + ~ChromeFrameAutomationProxyImpl(); virtual void SendAsAsync( IPC::SyncMessage* msg, SyncMessageReplyDispatcher::SyncMessageCallContext* context, void* key); + // Called on the worker thread. + virtual void OnChannelError(); + virtual void CancelAsync(void* key); virtual scoped_refptr<TabProxy> CreateTabProxy(int handle); @@ -81,73 +89,201 @@ class ChromeFrameAutomationProxyImpl : public ChromeFrameAutomationProxy, } protected: - explicit ChromeFrameAutomationProxyImpl(int launch_timeout); - ~ChromeFrameAutomationProxyImpl(); + friend class AutomationProxyCacheEntry; + ChromeFrameAutomationProxyImpl(AutomationProxyCacheEntry* entry, + int launch_timeout); + class CFMsgDispatcher; - scoped_refptr<CFMsgDispatcher> sync_; class TabProxyNotificationMessageFilter; + + scoped_refptr<CFMsgDispatcher> sync_; scoped_refptr<TabProxyNotificationMessageFilter> message_filter_; - friend class ProxyFactory; + AutomationProxyCacheEntry* proxy_entry_; }; -// This structure contains information used for launching chrome. -struct ChromeFrameLaunchParams { - int automation_server_launch_timeout; - GURL url; - GURL referrer; - FilePath profile_path; +// This class contains information used for launching chrome. +class ChromeFrameLaunchParams : // NOLINT + public base::RefCounted<ChromeFrameLaunchParams> { + public: + ChromeFrameLaunchParams(const GURL& url, const GURL& referrer, + const FilePath& profile_path, + const std::wstring& profile_name, + const std::wstring& extra_arguments, + bool incognito, bool widget_mode) + : launch_timeout_(kCommandExecutionTimeout), url_(url), + referrer_(referrer), profile_path_(profile_path), + profile_name_(profile_name), extra_arguments_(extra_arguments), + version_check_(true), incognito_mode_(incognito), + is_widget_mode_(widget_mode) { + } + + ~ChromeFrameLaunchParams() { + } + + void set_launch_timeout(int timeout) { + launch_timeout_ = timeout; + } + + int launch_timeout() const { + return launch_timeout_; + } + + const GURL& url() const { + return url_; + } + + void set_url(const GURL& url) { + url_ = url; + } + + const GURL& referrer() const { + return referrer_; + } + + void set_referrer(const GURL& referrer) { + referrer_ = referrer; + } + + const FilePath& profile_path() const { + return profile_path_; + } + + const std::wstring& profile_name() const { + return profile_name_; + } + + const std::wstring& extra_arguments() const { + return extra_arguments_; + } + + bool version_check() const { + return version_check_; + } + + void set_version_check(bool check) { + version_check_ = check; + } + + bool incognito() const { + return incognito_mode_; + } + + bool widget_mode() const { + return is_widget_mode_; + } + + protected: + int launch_timeout_; + GURL url_; + GURL referrer_; + FilePath profile_path_; + std::wstring profile_name_; + std::wstring extra_arguments_; + bool version_check_; + bool incognito_mode_; + bool is_widget_mode_; + + private: + DISALLOW_COPY_AND_ASSIGN(ChromeFrameLaunchParams); +}; + +// Callback when chrome process launch is complete and automation handshake +// (Hello message) is established. +struct DECLSPEC_NOVTABLE LaunchDelegate { // NOLINT + virtual void LaunchComplete(ChromeFrameAutomationProxy* proxy, + AutomationLaunchResult result) = 0; + virtual void AutomationServerDied() = 0; +}; // NOLINT + +// Manages a cached ChromeFrameAutomationProxyImpl entry and holds +// reference-less pointers to LaunchDelegate(s) to be notified in case +// of automation server process changes. +class AutomationProxyCacheEntry + : public base::RefCounted<AutomationProxyCacheEntry> { + public: + AutomationProxyCacheEntry(ChromeFrameLaunchParams* params, + LaunchDelegate* delegate); + + ~AutomationProxyCacheEntry(); + + void AddDelegate(LaunchDelegate* delegate); + void RemoveDelegate(LaunchDelegate* delegate, base::WaitableEvent* done, + bool* was_last_delegate); + + void StartSendUmaInterval(ChromeFrameHistogramSnapshots* snapshots, + int send_interval); + + DWORD WaitForThread(DWORD timeout) { // NOLINT + DCHECK(thread_.get()); + return ::WaitForSingleObject(thread_->thread_handle(), timeout); + } + + bool IsSameProfile(const std::wstring& name) const { + return lstrcmpiW(name.c_str(), profile_name.c_str()) == 0; + } + + base::Thread* thread() const { + return thread_.get(); + } + + MessageLoop* message_loop() const { + return thread_->message_loop(); + } + + bool IsSameThread(PlatformThreadId id) const { + return thread_->thread_id() == id; + } + + ChromeFrameAutomationProxyImpl* proxy() const { + DCHECK(IsSameThread(PlatformThread::CurrentId())); + return proxy_.get(); + } + + // Called by the proxy when the automation server has unexpectedly gone away. + void OnChannelError(); + + protected: + void CreateProxy(ChromeFrameLaunchParams* params, + LaunchDelegate* delegate); + void SendUMAData(); + + protected: std::wstring profile_name; - std::wstring extra_chrome_arguments; - bool perform_version_check; - bool incognito_mode; - bool is_widget_mode; + scoped_ptr<base::Thread> thread_; + scoped_ptr<ChromeFrameAutomationProxyImpl> proxy_; + AutomationLaunchResult launch_result_; + typedef std::vector<LaunchDelegate*> LaunchDelegates; + LaunchDelegates launch_delegates_; + // Used for UMA histogram logging to measure the time for the chrome + // automation server to start; + base::TimeTicks automation_server_launch_start_time_; + ChromeFrameHistogramSnapshots* snapshots_; + int uma_send_interval_; }; // We must create and destroy automation proxy in a thread with a message loop. // Hence thread cannot be a member of the proxy. class ProxyFactory { public: - // Callback when chrome process launch is complete and automation handshake - // (Hello message) is established. - struct DECLSPEC_NOVTABLE LaunchDelegate { // NOLINT - virtual void LaunchComplete(ChromeFrameAutomationProxy* proxy, - AutomationLaunchResult result) = 0; - }; // NOLINT - ProxyFactory(); virtual ~ProxyFactory(); + // Fetches or creates a new automation server instance. + // delegate may be NULL. If non-null, a pointer to the delegate will + // be stored for the lifetime of the automation process or until + // ReleaseAutomationServer is called. virtual void GetAutomationServer(LaunchDelegate* delegate, - const ChromeFrameLaunchParams& params, + ChromeFrameLaunchParams* params, void** automation_server_id); - virtual bool ReleaseAutomationServer(void* server_id); + virtual bool ReleaseAutomationServer(void* server_id, + LaunchDelegate* delegate); private: - struct ProxyCacheEntry { - std::wstring profile_name; - int ref_count; - scoped_ptr<base::Thread> thread; - ChromeFrameAutomationProxyImpl* proxy; - AutomationLaunchResult launch_result; - explicit ProxyCacheEntry(const std::wstring& profile); - }; - - void CreateProxy(ProxyCacheEntry* entry, - const ChromeFrameLaunchParams& params, - LaunchDelegate* delegate); - void ReleaseProxy(ProxyCacheEntry* entry, base::WaitableEvent* done); - - void SendUMAData(ProxyCacheEntry* proxy_entry); - - typedef StackVector<ProxyCacheEntry*, 4> Vector; + typedef StackVector<scoped_refptr<AutomationProxyCacheEntry>, 4> Vector; Vector proxies_; // Lock if we are going to call GetAutomationServer from more than one thread. Lock lock_; - // Used for UMA histogram logging to measure the time for the chrome - // automation server to start; - base::TimeTicks automation_server_launch_start_time_; - // Gathers histograms to be sent to Chrome. ChromeFrameHistogramSnapshots chrome_frame_histograms_; @@ -164,15 +300,16 @@ class ChromeFrameAutomationClient public base::RefCountedThreadSafe<ChromeFrameAutomationClient>, public PluginUrlRequestDelegate, public TabProxy::TabProxyDelegate, - public ProxyFactory::LaunchDelegate { + public LaunchDelegate { public: ChromeFrameAutomationClient(); ~ChromeFrameAutomationClient(); // Called from UI thread. virtual bool Initialize(ChromeFrameDelegate* chrome_frame_delegate, - const ChromeFrameLaunchParams& chrome_launch_params); + ChromeFrameLaunchParams* chrome_launch_params); void Uninitialize(); + void NotifyAndUninitialize(); virtual bool InitiateNavigation(const std::string& url, const std::string& referrer, @@ -284,7 +421,9 @@ class ChromeFrameAutomationClient protected: // ChromeFrameAutomationProxy::LaunchDelegate implementation. virtual void LaunchComplete(ChromeFrameAutomationProxy* proxy, - AutomationLaunchResult result); + AutomationLaunchResult result); + virtual void AutomationServerDied(); + // TabProxyDelegate implementation virtual void OnMessageReceived(TabProxy* tab, const IPC::Message& msg); virtual void OnChannelError(TabProxy* tab); @@ -301,12 +440,16 @@ class ChromeFrameAutomationClient Release(); } + scoped_refptr<ChromeFrameLaunchParams> launch_params() { + return chrome_launch_params_; + } + private: void OnMessageReceivedUIThread(const IPC::Message& msg); void OnChannelErrorUIThread(); HWND chrome_window() const { return chrome_window_; } - void BeginNavigate(const GURL& url, const GURL& referrer); + void BeginNavigate(); void BeginNavigateCompleted(AutomationMsg_NavigationResponseValues result); // Helpers @@ -352,7 +495,7 @@ class ChromeFrameAutomationClient // server being initialized. bool navigate_after_initialization_; - ChromeFrameLaunchParams chrome_launch_params_; + scoped_refptr<ChromeFrameLaunchParams> chrome_launch_params_; // When host network stack is used, this object is in charge of // handling network requests. diff --git a/chrome_frame/chrome_frame_npapi_unittest.cc b/chrome_frame/chrome_frame_npapi_unittest.cc index e845bb5..324fad1 100644 --- a/chrome_frame/chrome_frame_npapi_unittest.cc +++ b/chrome_frame/chrome_frame_npapi_unittest.cc @@ -81,7 +81,7 @@ class MockNPAPI: public ChromeFrameNPAPI { class MockAutomationClient: public ChromeFrameAutomationClient { public: MOCK_METHOD2(Initialize, bool(ChromeFrameDelegate*, - const ChromeFrameLaunchParams&)); + ChromeFrameLaunchParams*)); MOCK_METHOD1(SetEnableExtensionAutomation, void(const std::vector<std::string>&)); // NOLINT }; @@ -91,6 +91,16 @@ class MockProxyService: public NpProxyService { MOCK_METHOD2(Initialize, bool(NPP instance, ChromeFrameAutomationClient*)); }; +namespace { + +MATCHER_P4(LaunchParamEq, version_check, extra, incognito, widget, + "Basic check for ChromeFrameLaunchParams") { + return arg->version_check() == version_check && + arg->extra_arguments().compare(extra) == 0 && + arg->incognito() == incognito, + arg->widget_mode() == widget; +} +} // Test fixture to allow testing the privileged NPAPI APIs class TestNPAPIPrivilegedApi: public ::testing::Test { @@ -125,13 +135,12 @@ class TestNPAPIPrivilegedApi: public ::testing::Test { EXPECT_CALL(*mock_proxy, Initialize(_, _)).WillRepeatedly(Return(false)); + scoped_refptr<ChromeFrameLaunchParams> launch_params( + new ChromeFrameLaunchParams(GURL(), GURL(), FilePath(), profile_name, + extra_args, is_incognito, true)); + EXPECT_CALL(*mock_automation, - Initialize(_, AllOf( - Field(&ChromeFrameLaunchParams::perform_version_check, true), - Field(&ChromeFrameLaunchParams::extra_chrome_arguments, - StrEq(extra_args)), - Field(&ChromeFrameLaunchParams::incognito_mode, is_incognito), - Field(&ChromeFrameLaunchParams::is_widget_mode, true)))) + Initialize(_, LaunchParamEq(true, extra_args, is_incognito, true))) .WillOnce(Return(true)); if (expect_privilege_check) { @@ -554,3 +563,4 @@ TEST_F(TestNPAPIPrivilegedProperty, } // TODO(siggi): test invoking postPrivateMessage. + diff --git a/chrome_frame/chrome_frame_plugin.h b/chrome_frame/chrome_frame_plugin.h index 93461df..d334ab3 100644 --- a/chrome_frame/chrome_frame_plugin.h +++ b/chrome_frame/chrome_frame_plugin.h @@ -5,6 +5,9 @@ #ifndef CHROME_FRAME_CHROME_FRAME_PLUGIN_H_ #define CHROME_FRAME_CHROME_FRAME_PLUGIN_H_ +#include <string> +#include <vector> + #include "base/ref_counted.h" #include "base/win_util.h" #include "chrome_frame/chrome_frame_automation.h" @@ -61,23 +64,15 @@ END_MSG_MAP() bool incognito, bool is_widget_mode, const GURL& url, const GURL& referrer) { DCHECK(IsValid()); + DCHECK(launch_params_ == NULL); // We don't want to do incognito when privileged, since we're // running in browser chrome or some other privileged context. bool incognito_mode = !is_privileged_ && incognito; FilePath profile_path; GetProfilePath(profile_name, &profile_path); - ChromeFrameLaunchParams chrome_launch_params = { - kCommandExecutionTimeout, - url, - referrer, - profile_path, - profile_name, - extra_chrome_arguments, - true, - incognito_mode, - is_widget_mode - }; - return automation_client_->Initialize(this, chrome_launch_params); + launch_params_ = new ChromeFrameLaunchParams(url, referrer, profile_path, + profile_name, extra_chrome_arguments, incognito_mode, is_widget_mode); + return automation_client_->Initialize(this, launch_params_); } // ChromeFrameDelegate implementation @@ -240,6 +235,9 @@ END_MSG_MAP() // Our gateway to chrome land scoped_refptr<ChromeFrameAutomationClient> automation_client_; + // How we launched Chrome. + scoped_refptr<ChromeFrameLaunchParams> launch_params_; + // Url of the containing document. std::string document_url_; diff --git a/chrome_frame/test/automation_client_mock.cc b/chrome_frame/test/automation_client_mock.cc index d8a4cca..6ef792e 100644 --- a/chrome_frame/test/automation_client_mock.cc +++ b/chrome_frame/test/automation_client_mock.cc @@ -15,34 +15,39 @@ using testing::_; using testing::CreateFunctor; using testing::Return; -DISABLE_RUNNABLE_METHOD_REFCOUNT(ProxyFactory::LaunchDelegate); +DISABLE_RUNNABLE_METHOD_REFCOUNT(LaunchDelegate); DISABLE_RUNNABLE_METHOD_REFCOUNT(ChromeFrameAutomationClient); DISABLE_RUNNABLE_METHOD_REFCOUNT(chrome_frame_test::TimedMsgLoop); +MATCHER_P(LaunchParamProfileEq, profile_name, "Check for profile name") { + return arg->profile_name().compare(profile_name) == 0; +} + void MockProxyFactory::GetServerImpl(ChromeFrameAutomationProxy* pxy, void* proxy_id, AutomationLaunchResult result, LaunchDelegate* d, - const ChromeFrameLaunchParams& params, + ChromeFrameLaunchParams* params, void** automation_server_id) { *automation_server_id = proxy_id; Task* task = NewRunnableMethod(d, - &ProxyFactory::LaunchDelegate::LaunchComplete, pxy, result); + &LaunchDelegate::LaunchComplete, pxy, result); loop_->PostDelayedTask(FROM_HERE, task, - params.automation_server_launch_timeout/2); + params->launch_timeout() / 2); } void CFACMockTest::SetAutomationServerOk(int times) { EXPECT_CALL(factory_, GetAutomationServer(testing::NotNull(), - testing::Field(&ChromeFrameLaunchParams::profile_name, - testing::StrEq(profile_path_.BaseName().value())), - testing::NotNull())) + LaunchParamProfileEq(profile_path_.BaseName().value()), + testing::NotNull())) .Times(times) .WillRepeatedly(testing::Invoke(CreateFunctor(&factory_, &MockProxyFactory::GetServerImpl, get_proxy(), id_, AUTOMATION_SUCCESS))); - EXPECT_CALL(factory_, ReleaseAutomationServer(testing::Eq(id_))).Times(times); + EXPECT_CALL(factory_, + ReleaseAutomationServer(testing::Eq(id_), testing::NotNull())) + .Times(times); } void CFACMockTest::Set_CFD_LaunchFailed(AutomationLaunchResult result) { @@ -93,17 +98,12 @@ TEST(CFACWithChrome, CreateTooFast) { .Times(1) .WillOnce(QUIT_LOOP(loop)); - ChromeFrameLaunchParams clp = { - timeout, - GURL(), - GURL(), - profile_path, - profile_path.BaseName().value(), - L"", - false, - false, - false - }; + GURL empty; + scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams( + empty, empty, profile_path, profile_path.BaseName().value(), L"", + false, false)); + clp->set_launch_timeout(timeout); + clp->set_version_check(false); EXPECT_TRUE(client->Initialize(&cfd, clp)); loop.RunFor(10); client->Uninitialize(); @@ -130,17 +130,12 @@ TEST(CFACWithChrome, CreateNotSoFast) { EXPECT_CALL(cfd, OnAutomationServerLaunchFailed(_, _)) .Times(0); - ChromeFrameLaunchParams clp = { - timeout, - GURL(), - GURL(), - profile_path, - profile_path.BaseName().value(), - L"", - false, - false, - false - }; + GURL empty; + scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams( + empty, empty, profile_path, profile_path.BaseName().value(), L"", + false, false)); + clp->set_launch_timeout(timeout); + clp->set_version_check(false); EXPECT_TRUE(client->Initialize(&cfd, clp)); loop.RunFor(11); @@ -182,17 +177,12 @@ TEST(CFACWithChrome, NavigateOk) { .WillOnce(QUIT_LOOP(loop)); } - ChromeFrameLaunchParams clp = { - timeout, - GURL(), - GURL(), - profile_path, - profile_path.BaseName().value(), - L"", - false, - false, - false - }; + GURL empty; + scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams( + empty, empty, profile_path, profile_path.BaseName().value(), L"", + false, false)); + clp->set_launch_timeout(timeout); + clp->set_version_check(false); EXPECT_TRUE(client->Initialize(&cfd, clp)); loop.RunFor(10); client->Uninitialize(); @@ -233,17 +223,12 @@ TEST(CFACWithChrome, NavigateFailed) { .Times(1) .WillOnce(QUIT_LOOP_SOON(loop, 2)); - ChromeFrameLaunchParams clp = { - 10000, - GURL(), - GURL(), - profile_path, - profile_path.BaseName().value(), - L"", - false, - false, - false - }; + GURL empty; + scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams( + empty, empty, profile_path, profile_path.BaseName().value(), L"", + false, false)); + clp->set_launch_timeout(10000); + clp->set_version_check(false); EXPECT_TRUE(client->Initialize(&cfd, clp)); loop.RunFor(10); @@ -276,17 +261,12 @@ TEST_F(CFACMockTest, MockedCreateTabOk) { EXPECT_CALL(mock_proxy_, CancelAsync(_)).Times(testing::AnyNumber()); // Here we go! - ChromeFrameLaunchParams clp = { - timeout, - GURL(), - GURL(), - profile_path_, - profile_path_.BaseName().value(), - L"", - false, - false, - false - }; + GURL empty; + scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams( + empty, empty, profile_path_, profile_path_.BaseName().value(), L"", + false, false)); + clp->set_launch_timeout(timeout); + clp->set_version_check(false); EXPECT_TRUE(client_->Initialize(&cfd_, clp)); loop_.RunFor(10); @@ -313,17 +293,12 @@ TEST_F(CFACMockTest, MockedCreateTabFailed) { Set_CFD_LaunchFailed(AUTOMATION_CREATE_TAB_FAILED); // Here we go! - ChromeFrameLaunchParams clp = { - timeout_, - GURL(), - GURL(), - profile_path_, - profile_path_.BaseName().value(), - L"", - false, - false, - false - }; + GURL empty; + scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams( + empty, empty, profile_path_, profile_path_.BaseName().value(), L"", + false, false)); + clp->set_launch_timeout(timeout_); + clp->set_version_check(false); EXPECT_TRUE(client_->Initialize(&cfd_, clp)); loop_.RunFor(4); client_->Uninitialize(); @@ -333,7 +308,8 @@ class TestChromeFrameAutomationProxyImpl : public ChromeFrameAutomationProxyImpl { public: TestChromeFrameAutomationProxyImpl() - : ChromeFrameAutomationProxyImpl(1) { // 1 is an unneeded timeout. + // 1 is an unneeded timeout. + : ChromeFrameAutomationProxyImpl(NULL, 1) { } MOCK_METHOD3( SendAsAsync, @@ -357,17 +333,12 @@ TEST_F(CFACMockTest, OnChannelError) { TestChromeFrameAutomationProxyImpl proxy; returned_proxy_ = &proxy; - ChromeFrameLaunchParams clp = { - 1, // Unneeded timeout, but can't be 0. - GURL(), - GURL(), - profile_path_, - profile_path_.BaseName().value(), - L"", - false, - false, - false - }; + GURL empty; + scoped_refptr<ChromeFrameLaunchParams> clp(new ChromeFrameLaunchParams( + empty, empty, profile_path_, profile_path_.BaseName().value(), L"", + false, false)); + clp->set_launch_timeout(1); // Unneeded timeout, but can't be 0. + clp->set_version_check(false); HWND h1 = ::GetDesktopWindow(); HWND h2 = ::GetDesktopWindow(); diff --git a/chrome_frame/test/chrome_frame_automation_mock.h b/chrome_frame/test/chrome_frame_automation_mock.h index 141fb9c..dfa7900 100644 --- a/chrome_frame/test/chrome_frame_automation_mock.h +++ b/chrome_frame/test/chrome_frame_automation_mock.h @@ -32,17 +32,12 @@ class AutomationMockDelegate chrome_frame_test::GetProfilePath(profile_name)); automation_client_ = new ChromeFrameAutomationClient; - ChromeFrameLaunchParams clp = { - launch_timeout, - GURL(), - GURL(), - profile_path, - profile_name, - extra_chrome_arguments, - perform_version_check, - incognito, - is_widget_mode - }; + GURL empty; + scoped_refptr<ChromeFrameLaunchParams> clp( + new ChromeFrameLaunchParams(empty, empty, profile_path, profile_name, + extra_chrome_arguments, incognito, is_widget_mode)); + clp->set_launch_timeout(launch_timeout); + clp->set_version_check(perform_version_check); automation_client_->Initialize(this, clp); } ~AutomationMockDelegate() { diff --git a/chrome_frame/test/ie_event_sink.h b/chrome_frame/test/ie_event_sink.h index 48b5d30..ac1bf93 100644 --- a/chrome_frame/test/ie_event_sink.h +++ b/chrome_frame/test/ie_event_sink.h @@ -57,6 +57,14 @@ class IEEventListener { virtual void OnNewBrowserWindow(IDispatch* new_window, const wchar_t* url) {} }; +// Listener for IPropertyNotifySink. +class PropertyNotifySinkListener { + public: + virtual ~PropertyNotifySinkListener() {} + virtual void OnChanged(DISPID dispid) {} + virtual void OnRequestEdit(DISPID dispid) {} +}; + // This class sets up event sinks to the IWebBrowser interface. It forwards // all events to its listener. // TODO(kkania): Delete WebBrowserEventSink and use this class instead for @@ -229,6 +237,38 @@ END_SINK_MAP() static _ATL_FUNC_INFO kFileDownloadInfo; }; +class PropertyNotifySinkImpl + : public CComObjectRootEx<CComSingleThreadModel>, + public IPropertyNotifySink { + public: + PropertyNotifySinkImpl() : listener_(NULL) { + } + +BEGIN_COM_MAP(PropertyNotifySinkImpl) + COM_INTERFACE_ENTRY(IPropertyNotifySink) +END_COM_MAP() + + STDMETHOD(OnChanged)(DISPID dispid) { + if (listener_) + listener_->OnChanged(dispid); + return S_OK; + } + + STDMETHOD(OnRequestEdit)(DISPID dispid) { + if (listener_) + listener_->OnRequestEdit(dispid); + return S_OK; + } + + void set_listener(PropertyNotifySinkListener* listener) { + DCHECK(listener_ == NULL || listener == NULL); + listener_ = listener; + } + + protected: + PropertyNotifySinkListener* listener_; +}; + } // namespace chrome_frame_test #endif // CHROME_FRAME_TEST_IE_EVENT_SINK_H_ diff --git a/chrome_frame/test/mock_ie_event_sink_actions.h b/chrome_frame/test/mock_ie_event_sink_actions.h index 0ee8dd2..49d6d63 100644 --- a/chrome_frame/test/mock_ie_event_sink_actions.h +++ b/chrome_frame/test/mock_ie_event_sink_actions.h @@ -5,6 +5,8 @@ #ifndef CHROME_FRAME_TEST_MOCK_IE_EVENT_SINK_ACTIONS_H_ #define CHROME_FRAME_TEST_MOCK_IE_EVENT_SINK_ACTIONS_H_ +#include "base/scoped_bstr_win.h" +#include "chrome/common/chrome_switches.h" #include "chrome_frame/test/chrome_frame_test_utils.h" #include "chrome_frame/test/simulate_input.h" #include "testing/gmock/include/gmock/gmock.h" @@ -41,6 +43,28 @@ ACTION_P(ExpectRendererHasFocus, mock) { mock->event_sink()->ExpectRendererWindowHasFocus(); } +ACTION_P2(ConnectDocPropNotifySink, mock, sink) { + ScopedComPtr<IDispatch> document; + mock->event_sink()->web_browser2()->get_Document(document.Receive()); + EXPECT_TRUE(document != NULL); // NOLINT + if (document) { + sink->Attach(document); + } +} + +ACTION_P(DisconnectDocPropNotifySink, sink) { + sink->Detach(); +} + +ACTION_P3(DelayNavigateToCurrentUrl, mock, loop, delay) { + loop->PostDelayedTask(FROM_HERE, NewRunnableFunction(&NavigateToCurrentUrl, + mock), delay); +} + +ACTION_P2(ExpectDocumentReadystate, mock, ready_state) { + mock->ExpectDocumentReadystate(ready_state); +} + ACTION_P8(DelayExecCommand, mock, loop, delay, cmd_group_guid, cmd_id, cmd_exec_opt, in_args, out_args) { loop->PostDelayedTask(FROM_HERE, NewRunnableMethod(mock->event_sink(), @@ -106,6 +130,13 @@ ACTION_P4(DelaySendScanCode, loop, delay, c, mod) { simulate_input::SendScanCode, c, mod), delay); } + +ACTION(KillChromeFrameProcesses) { + KillAllNamedProcessesWithArgument( + UTF8ToWide(chrome_frame_test::kChromeImageName), + UTF8ToWide(switches::kChromeFrame)); +} + // This function selects the address bar via the Alt+d shortcut. This is done // via a delayed task which executes after the delay which is passed in. // The subsequent operations like typing in the actual url and then hitting @@ -138,10 +169,10 @@ ACTION_P5(ValidateWindowSize, mock, left, top, width, height) { int actual_height = 0; IWebBrowser2* web_browser2 = mock->event_sink()->web_browser2(); - web_browser2->get_Left(reinterpret_cast<long*>(&actual_left)); - web_browser2->get_Top(reinterpret_cast<long*>(&actual_top)); - web_browser2->get_Width(reinterpret_cast<long*>(&actual_width)); - web_browser2->get_Height(reinterpret_cast<long*>(&actual_height)); + web_browser2->get_Left(reinterpret_cast<long*>(&actual_left)); // NOLINT + web_browser2->get_Top(reinterpret_cast<long*>(&actual_top)); // NOLINT + web_browser2->get_Width(reinterpret_cast<long*>(&actual_width)); // NOLINT + web_browser2->get_Height(reinterpret_cast<long*>(&actual_height)); // NOLINT EXPECT_EQ(actual_left, left); EXPECT_EQ(actual_top, top); diff --git a/chrome_frame/test/mock_ie_event_sink_test.cc b/chrome_frame/test/mock_ie_event_sink_test.cc index ab1ac6d..fb1493a 100644 --- a/chrome_frame/test/mock_ie_event_sink_test.cc +++ b/chrome_frame/test/mock_ie_event_sink_test.cc @@ -6,6 +6,7 @@ #include <sstream> +#include "base/scoped_variant_win.h" #include "base/utf_string_conversions.h" #include "chrome_frame/test/mock_ie_event_sink_actions.h" @@ -140,6 +141,27 @@ void MockIEEventSink::ExpectAnyNavigations() { .Times(testing::AnyNumber()); } +void MockIEEventSink::ExpectDocumentReadystate(int ready_state) { + ScopedComPtr<IWebBrowser2> browser(event_sink_->web_browser2()); + EXPECT_TRUE(browser != NULL); + if (browser) { + ScopedComPtr<IDispatch> document; + browser->get_Document(document.Receive()); + EXPECT_TRUE(document != NULL); + if (document) { + DISPPARAMS params = { 0 }; + ScopedVariant result; + EXPECT_HRESULT_SUCCEEDED(document->Invoke(DISPID_READYSTATE, IID_NULL, + LOCALE_USER_DEFAULT, DISPATCH_PROPERTYGET, ¶ms, + result.Receive(), NULL, NULL)); + EXPECT_EQ(VT_I4, result.type()); + if (result.type() == VT_I4) { + EXPECT_EQ(ready_state, static_cast<int>(V_I4(&result))); + } + } + } +} + // MockIEEventSinkTest methods MockIEEventSinkTest::MockIEEventSinkTest() : server_mock_(1337, L"127.0.0.1", GetTestDataFolder()) { diff --git a/chrome_frame/test/mock_ie_event_sink_test.h b/chrome_frame/test/mock_ie_event_sink_test.h index 73e8778..b6e772a 100644 --- a/chrome_frame/test/mock_ie_event_sink_test.h +++ b/chrome_frame/test/mock_ie_event_sink_test.h @@ -114,6 +114,8 @@ class MockIEEventSink : public IEEventListener { // Expects any and all navigations. void ExpectAnyNavigations(); + void ExpectDocumentReadystate(int ready_state); + IEEventSink* event_sink() { return event_sink_; } private: @@ -147,6 +149,59 @@ class MockIEEventSink : public IEEventListener { CComObject<IEEventSink>* event_sink_; }; +// This mocks a PropertyNotifySinkListener, providing methods for +// expecting certain sequences of events. +class MockPropertyNotifySinkListener : public PropertyNotifySinkListener { + public: + MockPropertyNotifySinkListener() : cookie_(0), sink_(NULL) { + CComObject<PropertyNotifySinkImpl>::CreateInstance(&sink_); + sink_->AddRef(); + sink_->set_listener(this); + } + + ~MockPropertyNotifySinkListener() { + Detach(); + sink_->set_listener(NULL); + DLOG_IF(ERROR, sink_->m_dwRef != 1) + << "Event sink is still referenced externally: ref count = " + << sink_->m_dwRef; + sink_->Release(); + } + + // Override PropertyNotifySinkListener methods. + MOCK_METHOD1(OnChanged, void (DISPID dispid)); // NOLINT + + bool Attach(IUnknown* obj) { + DCHECK_EQ(cookie_, 0UL); + DCHECK(obj); + HRESULT hr = AtlAdvise(obj, sink_->GetUnknown(), IID_IPropertyNotifySink, + &cookie_); + if (SUCCEEDED(hr)) { + event_source_ = obj; + } else { + LOG(ERROR) << StringPrintf("AtlAdvise: 0x%08X", hr); + cookie_ = 0; + } + + return SUCCEEDED(hr); + } + + void Detach() { + if (event_source_) { + DCHECK_NE(cookie_, 0UL); + AtlUnadvise(event_source_, IID_IPropertyNotifySink, cookie_); + event_source_.Release(); + cookie_ = 0; + } + } + + private: + CComObject<PropertyNotifySinkImpl>* sink_; + DWORD cookie_; + ScopedComPtr<IUnknown> event_source_; +}; + + // Mocks a window observer so that tests can detect new windows. class MockWindowObserver : public WindowObserver { public: diff --git a/chrome_frame/test/proxy_factory_mock.cc b/chrome_frame/test/proxy_factory_mock.cc index 8c3040e..4e9ddbc 100644 --- a/chrome_frame/test/proxy_factory_mock.cc +++ b/chrome_frame/test/proxy_factory_mock.cc @@ -17,39 +17,43 @@ TEST(ProxyFactoryTest, CreateDestroy) { LaunchDelegateMock d; EXPECT_CALL(d, LaunchComplete(testing::NotNull(), testing::_)).Times(1); - ChromeFrameLaunchParams params; - params.automation_server_launch_timeout = 0; - params.profile_name = L"Adam.N.Epilinter"; - params.extra_chrome_arguments = L""; - params.perform_version_check = false; - params.incognito_mode = false; + GURL empty; + FilePath profile_path; + scoped_refptr<ChromeFrameLaunchParams> params( + new ChromeFrameLaunchParams(empty, empty, profile_path, + L"Adam.N.Epilinter", L"", false, false)); + params->set_launch_timeout(0); + params->set_version_check(false); void* id = NULL; f.GetAutomationServer(&d, params, &id); - f.ReleaseAutomationServer(id); + f.ReleaseAutomationServer(id, &d); } TEST(ProxyFactoryTest, CreateSameProfile) { ProxyFactory f; LaunchDelegateMock d; - EXPECT_CALL(d, LaunchComplete(testing::NotNull(), testing::_)).Times(2); + LaunchDelegateMock d2; + EXPECT_CALL(d, LaunchComplete(testing::NotNull(), testing::_)).Times(1); + EXPECT_CALL(d2, LaunchComplete(testing::NotNull(), testing::_)).Times(1); - ChromeFrameLaunchParams params; - params.automation_server_launch_timeout = 0; - params.profile_name = L"Dr. Gratiano Forbeson"; - params.extra_chrome_arguments = L""; - params.perform_version_check = false; - params.incognito_mode = false; + GURL empty; + FilePath profile_path; + scoped_refptr<ChromeFrameLaunchParams> params( + new ChromeFrameLaunchParams(empty, empty, profile_path, + L"Dr. Gratiano Forbeson", L"", false, false)); + params->set_launch_timeout(0); + params->set_version_check(false); void* i1 = NULL; void* i2 = NULL; f.GetAutomationServer(&d, params, &i1); - f.GetAutomationServer(&d, params, &i2); + f.GetAutomationServer(&d2, params, &i2); EXPECT_EQ(i1, i2); - f.ReleaseAutomationServer(i2); - f.ReleaseAutomationServer(i1); + f.ReleaseAutomationServer(i2, &d2); + f.ReleaseAutomationServer(i1, &d); } TEST(ProxyFactoryTest, CreateDifferentProfiles) { @@ -57,19 +61,19 @@ TEST(ProxyFactoryTest, CreateDifferentProfiles) { LaunchDelegateMock d; EXPECT_CALL(d, LaunchComplete(testing::NotNull(), testing::_)).Times(2); - ChromeFrameLaunchParams params1; - params1.automation_server_launch_timeout = 0; - params1.profile_name = L"Adam.N.Epilinter"; - params1.extra_chrome_arguments = L""; - params1.perform_version_check = false; - params1.incognito_mode = false; + GURL empty; + FilePath profile_path; + scoped_refptr<ChromeFrameLaunchParams> params1( + new ChromeFrameLaunchParams(empty, empty, profile_path, + L"Adam.N.Epilinter", L"", false, false)); + params1->set_launch_timeout(0); + params1->set_version_check(false); - ChromeFrameLaunchParams params2; - params2.automation_server_launch_timeout = 0; - params2.profile_name = L"Dr. Gratiano Forbeson"; - params2.extra_chrome_arguments = L""; - params2.perform_version_check = false; - params2.incognito_mode = false; + scoped_refptr<ChromeFrameLaunchParams> params2( + new ChromeFrameLaunchParams(empty, empty, profile_path, + L"Dr. Gratiano Forbeson", L"", false, false)); + params2->set_launch_timeout(0); + params2->set_version_check(false); void* i1 = NULL; void* i2 = NULL; @@ -78,8 +82,8 @@ TEST(ProxyFactoryTest, CreateDifferentProfiles) { f.GetAutomationServer(&d, params2, &i2); EXPECT_NE(i1, i2); - f.ReleaseAutomationServer(i2); - f.ReleaseAutomationServer(i1); + f.ReleaseAutomationServer(i2, &d); + f.ReleaseAutomationServer(i1, &d); } TEST(ProxyFactoryTest, FastCreateDestroy) { @@ -87,17 +91,17 @@ TEST(ProxyFactoryTest, FastCreateDestroy) { LaunchDelegateMock* d1 = new LaunchDelegateMock(); LaunchDelegateMock* d2 = new LaunchDelegateMock(); - ChromeFrameLaunchParams params; - params.automation_server_launch_timeout = 10000; - params.profile_name = L"Dr. Gratiano Forbeson"; - params.extra_chrome_arguments = L""; - params.perform_version_check = false; - params.incognito_mode = false; + GURL empty; + FilePath profile_path; + scoped_refptr<ChromeFrameLaunchParams> params( + new ChromeFrameLaunchParams(empty, empty, profile_path, + L"Dr. Gratiano Forbeson", L"", false, false)); + params->set_launch_timeout(10000); + params->set_version_check(false); void* i1 = NULL; base::WaitableEvent launched(true, false); EXPECT_CALL(*d1, LaunchComplete(testing::NotNull(), AUTOMATION_SUCCESS)) - .Times(1) .WillOnce(testing::InvokeWithoutArgs(&launched, &base::WaitableEvent::Signal)); f.GetAutomationServer(d1, params, &i1); @@ -114,10 +118,10 @@ TEST(ProxyFactoryTest, FastCreateDestroy) { void* i2 = NULL; f.GetAutomationServer(d2, params, &i2); EXPECT_EQ(i1, i2); - f.ReleaseAutomationServer(i2); + f.ReleaseAutomationServer(i2, d2); delete d2; ::SetThreadPriority(::GetCurrentThread(), THREAD_PRIORITY_NORMAL); - f.ReleaseAutomationServer(i1); + f.ReleaseAutomationServer(i1, d1); delete d1; } diff --git a/chrome_frame/test/proxy_factory_mock.h b/chrome_frame/test/proxy_factory_mock.h index 8f2bb2e..609d4b6 100644 --- a/chrome_frame/test/proxy_factory_mock.h +++ b/chrome_frame/test/proxy_factory_mock.h @@ -1,8 +1,8 @@ // Copyright (c) 2006-2010 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_FRAME_PROXY_FACTORY_MOCK_H_ -#define CHROME_FRAME_PROXY_FACTORY_MOCK_H_ +#ifndef CHROME_FRAME_TEST_PROXY_FACTORY_MOCK_H_ +#define CHROME_FRAME_TEST_PROXY_FACTORY_MOCK_H_ #include <windows.h> #include <string> @@ -10,16 +10,20 @@ #include "gmock/gmock.h" #include "chrome_frame/chrome_frame_automation.h" -struct LaunchDelegateMock : public ProxyFactory::LaunchDelegate { +struct LaunchDelegateMock : public LaunchDelegate { MOCK_METHOD2(LaunchComplete, void(ChromeFrameAutomationProxy*, - AutomationLaunchResult)); + AutomationLaunchResult)); + MOCK_METHOD0(AutomationServerDied, void()); }; class MockProxyFactory : public ProxyFactory { public: - MOCK_METHOD3(GetAutomationServer, void (ProxyFactory::LaunchDelegate*, - const ChromeFrameLaunchParams& params, void** automation_server_id)); - MOCK_METHOD1(ReleaseAutomationServer, bool(void* id)); + MOCK_METHOD3(GetAutomationServer, + void (LaunchDelegate*, // NOLINT + ChromeFrameLaunchParams* params, + void** automation_server_id)); + MOCK_METHOD2(ReleaseAutomationServer, bool(void* server_id, + LaunchDelegate* delegate)); MockProxyFactory() : thread_("mock factory worker") { thread_.Start(); @@ -31,7 +35,7 @@ class MockProxyFactory : public ProxyFactory { void* proxy_id, AutomationLaunchResult result, LaunchDelegate* d, - const ChromeFrameLaunchParams& params, + ChromeFrameLaunchParams* params, void** automation_server_id); base::Thread thread_; @@ -39,5 +43,5 @@ class MockProxyFactory : public ProxyFactory { }; -#endif // CHROME_FRAME_PROXY_FACTORY_MOCK_H_ +#endif // CHROME_FRAME_TEST_PROXY_FACTORY_MOCK_H_ diff --git a/chrome_frame/test/ui_test.cc b/chrome_frame/test/ui_test.cc index 143df63..8eb4475 100644 --- a/chrome_frame/test/ui_test.cc +++ b/chrome_frame/test/ui_test.cc @@ -11,6 +11,8 @@ #include "chrome_frame/test/mock_ie_event_sink_actions.h" #include "chrome_frame/test/mock_ie_event_sink_test.h" +#include "testing/gmock_mutant.h" + using testing::_; using testing::InSequence; using testing::StrCaseEq; @@ -235,6 +237,84 @@ TEST_P(FullTabUITest, FLAKY_ViewSource) { LaunchIEAndNavigate(GetSimplePageUrl()); } +void NavigateToCurrentUrl(MockIEEventSink* mock) { + IWebBrowser2* browser = mock->event_sink()->web_browser2(); + DCHECK(browser); + ScopedBstr bstr; + HRESULT hr = browser->get_LocationURL(bstr.Receive()); + EXPECT_HRESULT_SUCCEEDED(hr); + if (SUCCEEDED(hr)) { + DCHECK(bstr.Length()); + VARIANT empty = ScopedVariant::kEmptyVariant; + hr = browser->Navigate(bstr, &empty, &empty, &empty, &empty); + EXPECT_HRESULT_SUCCEEDED(hr); + } +} + +// Tests that Chrome gets re-instantiated after crash if we reload via +// the address bar or via a new navigation. +TEST_P(FullTabUITest, TabCrashReload) { + using testing::DoAll; + + if (!GetParam().invokes_cf()) { + LOG(ERROR) << "Test needs CF."; + return; + } + + MockPropertyNotifySinkListener prop_listener; + InSequence expect_in_sequence_for_scope; + + EXPECT_CALL(ie_mock_, OnLoad(_, StrEq(GetSimplePageUrl()))) + .WillOnce(DoAll( + ExpectRendererHasFocus(&ie_mock_), + ExpectDocumentReadystate(&ie_mock_, READYSTATE_COMPLETE), + ConnectDocPropNotifySink(&ie_mock_, &prop_listener), + KillChromeFrameProcesses())); + + EXPECT_CALL(prop_listener, OnChanged(DISPID_READYSTATE)) + .WillOnce(DoAll( + ExpectDocumentReadystate(&ie_mock_, READYSTATE_UNINITIALIZED), + DelayNavigateToCurrentUrl(&ie_mock_, &loop_, 10))); + + EXPECT_CALL(ie_mock_, OnLoad(_, StrEq(GetSimplePageUrl()))) + .WillOnce(CloseBrowserMock(&ie_mock_)); + + LaunchIEAndNavigate(GetSimplePageUrl()); +} + +// Tests if Chrome gets restarted after a crash by just refreshing the document. +TEST_P(FullTabUITest, TabCrashRefresh) { + using testing::DoAll; + + if (!GetParam().invokes_cf()) { + LOG(ERROR) << "Test needs CF."; + return; + } + + MockPropertyNotifySinkListener prop_listener; + InSequence expect_in_sequence_for_scope; + + EXPECT_CALL(ie_mock_, OnLoad(_, StrEq(GetSimplePageUrl()))) + .WillOnce(DoAll( + ExpectRendererHasFocus(&ie_mock_), + ExpectDocumentReadystate(&ie_mock_, READYSTATE_COMPLETE), + ConnectDocPropNotifySink(&ie_mock_, &prop_listener), + KillChromeFrameProcesses())); + + VARIANT empty = ScopedVariant::kEmptyVariant; + EXPECT_CALL(prop_listener, OnChanged(/*DISPID_READYSTATE*/_)) + .WillOnce(DoAll( + DisconnectDocPropNotifySink(&prop_listener), + ExpectDocumentReadystate(&ie_mock_, READYSTATE_UNINITIALIZED), + DelayExecCommand(&ie_mock_, &loop_, 10, static_cast<GUID*>(NULL), + OLECMDID_REFRESH, 0, &empty, &empty))); + + EXPECT_CALL(ie_mock_, OnLoad(_, StrEq(GetSimplePageUrl()))) + .WillOnce(CloseBrowserMock(&ie_mock_)); + + LaunchIEAndNavigate(GetSimplePageUrl()); +} + // Test fixture for tests related to the context menu UI. Since the context // menus for CF and IE are different, these tests are not parameterized. class ContextMenuTest : public MockIEEventSinkTest, public testing::Test { diff --git a/chrome_frame/test_utils.cc b/chrome_frame/test_utils.cc index 95c35af..fb46998 100644 --- a/chrome_frame/test_utils.cc +++ b/chrome_frame/test_utils.cc @@ -297,7 +297,6 @@ bool KillAllNamedProcessesWithArgument(const std::wstring& process_name, base::NamedProcessIterator iter(process_name, &filter); while (const base::ProcessEntry* entry = iter.NextProcessEntry()) { if (!base::KillProcessById(entry->pid(), 0, true)) { - DLOG(ERROR) << "Failed to kill process " << (*entry).th32ProcessID; result = false; } } diff --git a/chrome_frame/urlmon_url_request.cc b/chrome_frame/urlmon_url_request.cc index 47f34ac..08f44ff 100644 --- a/chrome_frame/urlmon_url_request.cc +++ b/chrome_frame/urlmon_url_request.cc @@ -184,7 +184,7 @@ void UrlmonUrlRequest::TerminateBind(TerminateBindCallback* callback) { size_t UrlmonUrlRequest::SendDataToDelegate(size_t bytes_to_read) { DCHECK_EQ(thread_, PlatformThread::CurrentId()); - DCHECK(id() != -1); + DCHECK_NE(id(), -1); DCHECK_GT(bytes_to_read, 0U); size_t bytes_copied = 0; if (delegate_) { @@ -220,7 +220,7 @@ size_t UrlmonUrlRequest::SendDataToDelegate(size_t bytes_to_read) { if (bytes_copied) { ++calling_delegate_; - DCHECK(id() != -1); + DCHECK_NE(id(), -1); delegate_->OnReadComplete(id(), read_data); --calling_delegate_; } @@ -375,7 +375,7 @@ STDMETHODIMP UrlmonUrlRequest::OnStopBinding(HRESULT result, LPCWSTR error) { // TRUE |TRUE => Something went wrong!! if (pending_data_) { - DCHECK(pending_read_size_ == 0); + DCHECK_EQ(pending_read_size_, 0UL); ReleaseBindings(); return S_OK; } @@ -619,7 +619,7 @@ STDMETHODIMP UrlmonUrlRequest::OnResponse(DWORD dwResponseCode, // Inform the delegate. headers_received_ = true; - DCHECK(id() != -1); + DCHECK_NE(id(), -1); delegate_->OnResponseStarted(id(), "", // mime_type raw_headers.c_str(), // headers @@ -908,8 +908,10 @@ void UrlmonUrlRequestManager::StartRequest(int request_id, DLOG(INFO) << __FUNCTION__ << " id: " << request_id; DCHECK_EQ(0, calling_delegate_); - if (stopping_) + if (stopping_) { + DLOG(WARNING) << __FUNCTION__ << " request not started (stopping)"; return; + } DCHECK(request_map_.find(request_id) == request_map_.end()); DCHECK(GURL(request_info.url).is_valid()); diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc index bed5409..c92c0f1 100644 --- a/chrome_frame/utils.cc +++ b/chrome_frame/utils.cc @@ -1175,7 +1175,12 @@ std::string BindStatus2Str(ULONG bind_status) { } std::string PiFlags2Str(DWORD flags) { -#define ADD_PI_FLAG(x) if (flags & x) { s.append(#x ## " "); flags &= ~x; } +#define ADD_PI_FLAG(x) \ + if (flags & x) { \ + s.append(#x ## " "); \ + flags &= ~x; \ + } + std::string s = " flags "; ADD_PI_FLAG(PI_PARSE_URL); ADD_PI_FLAG(PI_FILTER_MODE); @@ -1200,7 +1205,12 @@ std::string PiFlags2Str(DWORD flags) { } std::string Bscf2Str(DWORD flags) { -#define ADD_BSCF_FLAG(x) if (flags & x) { s.append(#x ## " "); flags &= ~x; } +#define ADD_BSCF_FLAG(x) \ + if (flags & x) {\ + s.append(#x ## " "); \ + flags &= ~x; \ + } + std::string s = " flags "; ADD_BSCF_FLAG(BSCF_FIRSTDATANOTIFICATION) ADD_BSCF_FLAG(BSCF_INTERMEDIATEDATANOTIFICATION) @@ -1253,8 +1263,7 @@ bool ChromeFrameUrl::Parse(const std::wstring& url) { attach_to_external_tab_ = MatchPatternWide(url.c_str(), kChromeFrameAttachTabPattern); - is_chrome_protocol_ = StartsWith(url, kChromeProtocolPrefix, - false); + is_chrome_protocol_ = StartsWith(url, kChromeProtocolPrefix, false); DCHECK(!(attach_to_external_tab_ && is_chrome_protocol_)); if (is_chrome_protocol_) { url_.erase(0, lstrlen(kChromeProtocolPrefix)); |