Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: src/plugin/PluginClass.cpp

Issue 29332848: Issue #3432 - Manage COM events with a resource class
Patch Set: rebase, address comments Created Jan. 7, 2016, 4:21 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« src/plugin/PluginClass.h ('K') | « src/plugin/PluginClass.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/plugin/PluginClass.cpp
===================================================================
--- a/src/plugin/PluginClass.cpp
+++ b/src/plugin/PluginClass.cpp
@@ -90,7 +90,8 @@
}
CPluginClass::CPluginClass()
- : m_webBrowser2(nullptr)
+ : m_webBrowser2(nullptr),
+ detachedInitializationFailed(false)
{
DEBUG_GENERAL([this]() -> std::wstring
{
@@ -102,7 +103,6 @@
//Use this line to debug memory leaks
// _CrtDumpMemoryLeaks();
- m_isAdvised = false;
m_hTabWnd = NULL;
m_hStatusBarWnd = NULL;
m_hPaneWnd = NULL;
@@ -187,11 +187,11 @@
{
if (thisPtr == NULL)
return 0;
- if (!((CPluginClass*)thisPtr)->InitObject())
+ auto self = static_cast<CPluginClass*>(thisPtr);
+ if (!self->InitObject())
{
- ((CPluginClass*)thisPtr)->Unadvise();
+ self->detachedInitializationFailed = true;
}
-
return 0;
}
@@ -218,7 +218,6 @@
{
DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_COINIT, "Class::SetSite - CoInitialize");
}
-
/*
* We were instantiated as a BHO, so our site is always of type IWebBrowser2.
*/
@@ -249,34 +248,22 @@
try
{
- HRESULT hr = DispEventAdvise(m_webBrowser2);
- if (SUCCEEDED(hr))
- {
- m_isAdvised = true;
- try
- {
- std::thread startInitObjectThread(StartInitObject, this);
- startInitObjectThread.detach(); // TODO: but actually we should wait for the thread in the dtr.
- }
- catch (const std::system_error& ex)
- {
- DEBUG_SYSTEM_EXCEPTION(ex, PLUGIN_ERROR_THREAD, PLUGIN_ERROR_MAIN_THREAD_CREATE_PROCESS,
- "Class::Thread - Failed to create StartInitObject thread");
- }
- }
- else
- {
- DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_ADVICE, "Class::SetSite - Advise");
- }
+ std::thread startInitObjectThread(StartInitObject, this);
+ startInitObjectThread.detach(); // TODO: but actually we should wait for the thread in the dtr.
}
- catch (const std::runtime_error& ex)
+ catch (const std::system_error& ex)
{
- DEBUG_EXCEPTION(ex);
- Unadvise();
+ detachedInitializationFailed = true;
+ DEBUG_SYSTEM_EXCEPTION(ex, PLUGIN_ERROR_THREAD, PLUGIN_ERROR_MAIN_THREAD_CREATE_PROCESS,
+ "Class::Thread - Failed to create StartInitObject thread");
}
+
+ // Start events last to alleviate issues with deferred evaluation
+ browserEvents.Start(this, m_webBrowser2);
}
else
{
+ browserEvents.Stop();
DEBUG_GENERAL([this]() -> std::wstring
{
std::wstringstream ss;
@@ -285,7 +272,6 @@
return ss.str();
}());
- Unadvise();
// Destroy window
if (m_pWndProcStatus)
@@ -465,6 +451,8 @@
{
try
{
+ if (detachedInitializationFailed) return;
+
ATL::CComQIPtr<IWebBrowser2> webBrowser = frameBrowserDisp;
if (!webBrowser)
{
@@ -509,6 +497,7 @@
// Entry point
void STDMETHODCALLTYPE CPluginClass::OnDownloadComplete()
{
+ if (detachedInitializationFailed) return;
try
{
if (!m_webBrowser2)
@@ -527,6 +516,7 @@
// Entry point
void STDMETHODCALLTYPE CPluginClass::OnDocumentComplete(IDispatch* frameBrowserDisp, VARIANT* /*urlOrPidl*/)
{
+ if (detachedInitializationFailed) return;
try
{
DEBUG_NAVI(L"Navi::Document Complete");
@@ -546,6 +536,7 @@
// Entry point
void STDMETHODCALLTYPE CPluginClass::OnWindowStateChanged(unsigned long flags, unsigned long validFlagsMask)
{
+ if (detachedInitializationFailed) return;
try
{
DEBUG_GENERAL(L"WindowStateChanged (check tab changed)");
@@ -576,6 +567,7 @@
// Entry point
void STDMETHODCALLTYPE CPluginClass::OnCommandStateChange(long /*command*/, VARIANT_BOOL /*enable*/)
{
+ if (detachedInitializationFailed) return;
try
{
if (m_hPaneWnd == NULL)
@@ -601,18 +593,6 @@
}
}
-// Entry point
-void STDMETHODCALLTYPE CPluginClass::OnOnQuit()
-{
- try
- {
- Unadvise();
- }
- catch (...)
- {
- }
-}
-
bool CPluginClass::InitObject()
{
DEBUG_GENERAL("InitObject - begin");
@@ -1001,6 +981,7 @@
// Entry point
STDMETHODIMP CPluginClass::QueryStatus(const GUID* pguidCmdGroup, ULONG cCmds, OLECMD prgCmds[], OLECMDTEXT* pCmdText)
{
+ if (detachedInitializationFailed) return S_OK;
try
{
if (cCmds == 0) return E_INVALIDARG;
@@ -1217,6 +1198,7 @@
// Entry point
STDMETHODIMP CPluginClass::Exec(const GUID*, DWORD nCmdID, DWORD, VARIANTARG*, VARIANTARG*)
{
+ if (detachedInitializationFailed) return S_OK;
try
{
HWND hBrowserWnd = GetBrowserHWND();
@@ -1642,29 +1624,6 @@
}
}
-
-void CPluginClass::Unadvise()
-{
- if (!m_webBrowser2)
- {
- DEBUG_ERROR_LOG(0, 0, 0, "CPluginClass::Unadvise - Reached with m_webBrowser2 == nullptr");
- return;
- }
- s_criticalSectionLocal.Lock();
- {
- if (m_isAdvised)
- {
- HRESULT hr = DispEventUnadvise(m_webBrowser2);
- if (FAILED(hr))
- {
- DEBUG_ERROR_LOG(hr, PLUGIN_ERROR_SET_SITE, PLUGIN_ERROR_SET_SITE_UNADVISE, "Class::Unadvise - Unadvise");
- }
- m_isAdvised = false;
- }
- }
- s_criticalSectionLocal.Unlock();
-}
-
HICON CPluginClass::GetIcon(int type)
{
HICON icon = NULL;
« src/plugin/PluginClass.h ('K') | « src/plugin/PluginClass.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld