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 84 matching lines @@
 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;
-  bool IsRootPageBrowser(IWebBrowser2*);
+  bool IsRootBrowser(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();
 
-  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.

+  /**
+   * A browser interface pointer to our site object
+   *
+   * It's values are set and reset solely in SetSite().
+   */
+  CComPtr<IWebBrowser2> m_webBrowser2;
   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 26 matching lines @@
 
   // Async browser
   static CComQIPtr<IWebBrowser2> s_asyncWebBrowser2;
   static CComQIPtr<IWebBrowser2> GetAsyncBrowser();
 };
 
 OBJECT_ENTRY_AUTO(__uuidof(PluginClass), CPluginClass)
 
 
 #endif // _PLUGIN_CLASS_H_