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 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();
 
-public:
-  CComQIPtr<IWebBrowser2> m_webBrowser2;
-private:
+  /**
+   * Our site object as a browser.
+   *
+   * It's values are set and reset solely in SetSite().

[sergei, 2015/11/30 15:52:09]

This part of the comment is not necessary, it's clear from the aim of
ATL::CComPtr.

[Eric, 2015/11/30 16:31:51]

I disagree. Nothing about this variable is either clear or obvious.

[Oleksandr, 2015/12/03 11:51:58]

I assume we're talking about the part of the comment below, not above. I agree
it looks redundant - CComPtr usage is always a defensive programming. But this
being just a comment I don't mind leaving it here.

[Eric, 2015/12/03 14:24:54]

It's no more defensive, at least ordinarily, than using 'unique_ptr' or
'shared_ptr' is.

In the present case it's compensating for a poor interface design in SetSite(),
which takes an 'IUnknown*' and contains no further type information, even though
it's always present in the caller.

I've edited down the comment.

+   * The declaration as CComPtr is defensive programming;
+   *   it ensures that the site keeps this interface alive for us in the
+   *   unlikely case that the site is different from a browser.
+   */
+  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 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_