Issue #3383 - Rewrite and simplify browser-site handling in CPluginClass

src/plugin/PluginClass.h

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 61 matching lines @@
     SINK_ENTRY_EX(1, DIID_DWebBrowserEvents2, DISPID_DOWNLOADCOMPLETE, OnDownloadComplete)
     SINK_ENTRY_EX(1, DIID_DWebBrowserEvents2, DISPID_DOCUMENTCOMPLETE, OnDocumentComplete)
     SINK_ENTRY_EX(1, DIID_DWebBrowserEvents2, DISPID_WINDOWSTATECHANGED, OnWindowStateChanged)
     SINK_ENTRY_EX(1, DIID_DWebBrowserEvents2, DISPID_COMMANDSTATECHANGE, OnCommandStateChange)
     SINK_ENTRY_EX(1, DIID_DWebBrowserEvents2, DISPID_ONQUIT, OnOnQuit)
   END_SINK_MAP()
 
   CPluginClass();
   ~CPluginClass();
 
-  HRESULT FinalConstruct();
-  void FinalRelease();
-
   // IObjectWithSite
-
   STDMETHOD(SetSite)(IUnknown *pUnkSite);
 
   // IOleCommandTarget
-
   STDMETHOD(QueryStatus)(const GUID* pguidCmdGroup, ULONG cCmds, OLECMD prgCmds[], OLECMDTEXT* pCmdText);
   STDMETHOD(Exec)(const GUID*, DWORD nCmdID, DWORD, VARIANTARG*, VARIANTARG* pvaOut);
 
 
   static CPluginTab* GetTab(DWORD dwThreadId);
   CPluginTab* GetTab();
 
   void UpdateStatusBar();
-  static DWORD WINAPI MainThreadProc(LPVOID pParam);
 
 private:
 
   bool SetMenuBar(HMENU hMenu, const std::wstring& url);
   HMENU CreatePluginMenu(const std::wstring& url);
 
   void DisplayPluginMenu(HMENU hMenu, int nToolbarCmdID, POINT pt, UINT nMenuFlags);
   bool CreateStatusBarPane();
 
 public:
   HWND GetBrowserHWND() const;
-  HWND GetTabHWND() const;
-  CComQIPtr<IWebBrowser2> GetBrowser() const;
-
+  bool IsRootPageBrowser(IWebBrowser2*);
 
   static CPluginMimeFilterClient* s_mimeFilter;
 
 private:
 
   std::wstring GetBrowserUrl() const;
 
   static DWORD WINAPI StartInitObject(LPVOID thisPtr);
   bool InitObject();
   void CloseTheme();
@@ skipping 15 matching lines @@
   void STDMETHODCALLTYPE OnDownloadComplete();
   void STDMETHODCALLTYPE OnDocumentComplete(IDispatch* frameBrowserDisp, VARIANT* /*urlOrPidl*/);
   void STDMETHODCALLTYPE OnWindowStateChanged(unsigned long flags, unsigned long validFlagsMask);
   void STDMETHODCALLTYPE OnCommandStateChange(long command, VARIANT_BOOL enable);
   void STDMETHODCALLTYPE OnOnQuit();
   void Unadvise();
 
   void ShowStatusBar();
   bool IsStatusBarEnabled();
 
-public:
-  CComQIPtr<IWebBrowser2> m_webBrowser2;
-private:
+  IWebBrowser2* m_webBrowser2;

[sergei, 2015/10/01 16:15:51]

I really don't understand the reluctance to use smart pointers. The one point
here is that it should be without QI.

[Oleksandr, 2015/10/05 10:44:47]

+1.

[Eric, 2015/11/18 13:57:31]

My rule: Use them when you need them and don't use them when you don't need
them. In addition to a performance improvement, the code expresses intent
better.

Using CComPtr object as a class member clearly expresses an intent that it's
holding a reference to a COM object (by means of its reference increment) so
that the COM object is not destroyed. This is relevant _only_ when the lifespan
of the COM object does not encompass the lifetime of the member.

Assertion: We don't need CComPtr here because the IWebBrowser2 object
encompasses that of CPluginClass.

The IWebBrowser2 COM object in IE is the one that instantiates the BHO object
(CPluginClass) through the factory method. Since the IWebBrowser2 object
correctly uses SetSite(), we'll receive both a call to SetSite(non-nullptr)
before it calls anything else and a call to SetSite(0) after anything else and
before it's destroyed. Hence the life span of the IWebBrowser2 completely
encompasses that of CPluginClass.

   HWND m_hBrowserWnd;
   HWND m_hTabWnd;
   HWND m_hStatusBarWnd;
   HWND m_hPaneWnd;
   
   WNDPROC m_pWndProcStatus;
   int m_nPaneWidth;
   HANDLE m_hTheme;
 
   CriticalSection m_csStatusBar;
@@ skipping 11 matching lines @@
   // Icons
   static HICON s_hIcons[ICON_MAX];
   static DWORD s_hIconTypes[ICON_MAX];
 
   static HICON GetIcon(int type);
 
   // Main thread
   static HANDLE s_hMainThread;
   static bool s_isMainThreadDone;
 
-  static HANDLE GetMainThreadHandle();
-  static bool IsMainThreadDone(HANDLE mainThread);
-
   static HINSTANCE s_hUxtheme;
   static std::set<CPluginClass*> s_instances;
   static std::map<DWORD,CPluginClass*> s_threadInstances;
   static CComAutoCriticalSection s_criticalSectionLocal;
-  static CComAutoCriticalSection s_criticalSectionBrowser;
   static CComAutoCriticalSection s_criticalSectionWindow;
 
   // Async browser
   static CComQIPtr<IWebBrowser2> s_asyncWebBrowser2;
   static CComQIPtr<IWebBrowser2> GetAsyncBrowser();
 };
 
 OBJECT_ENTRY_AUTO(__uuidof(PluginClass), CPluginClass)
 
 
 #endif // _PLUGIN_CLASS_H_