Manually instantiate the BHO at need from the Toolband.

Enabling (either through IE's plugin manager or via its context menu) a visual gadget depending 
on ceee would create and display that gadget, but the BHO would not be instantiated
(the latter part is consistent with documented IE BHO behaviur). The new mechanism will allow
a dependant CF gadget to make sure the BHO it requries already exists and take action (for instance-
create the BHO) when required. 
The pattern for this behaviour is taken (partly) from chrome_frame_activex.cpp.

Submitting for motek@google.com. Original review at http://codereview.chromium.org/4918001/.

BUG=3019459
TEST=none

Review URL: http://codereview.chromium.org/5227001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@66640 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 225 insertions, 5 deletions

diff --git a/ceee/common/com_utils.cc b/ceee/common/com_utils.cc
index b251ca1..c3b8bf3 100644
--- a/ceee/common/com_utils.cc
+++ b/ceee/common/com_utils.cc
@@ -76,4 +76,19 @@ HRESULT ModuleRegistrationWithoutAppid(int reg_id, BOOL should_register) {
   return UpdateRegistryFromResourceImpl(reg_id, should_register, entries);
 }
 
+bool GuidToString(const GUID& id, std::wstring* guid_as_text) {
+  DCHECK(guid_as_text != NULL);
+  if (guid_as_text == NULL)
+    return false;
+
+  wchar_t id_as_string[MAX_PATH] = {0};
+  if (::StringFromGUID2(id, id_as_string, arraysize(id_as_string)) > 0) {
+    *guid_as_text = id_as_string;
+    return true;
+  }
+
+  NOTREACHED() << "Could not convert GUID to string.";
+  return true;
+}
+
 }  // namespace com
diff --git a/ceee/common/com_utils.h b/ceee/common/com_utils.h
index e531ba4..30df2ae 100644
--- a/ceee/common/com_utils.h
+++ b/ceee/common/com_utils.h
@@ -88,6 +88,10 @@ std::ostream& operator<<(std::ostream& os, const LogWe& we);
 // replaceables, but no %APPID%.
 HRESULT ModuleRegistrationWithoutAppid(int reg_id, BOOL should_register);
 
+// Converts GUID into text and places in the provided object. Returns true if
+// succeeded. Logs possible error codes.
+bool GuidToString(const GUID& id, std::wstring* guid_as_text);
+
 }  // namespace com
 
 #endif  // CEEE_COMMON_COM_UTILS_H_
diff --git a/ceee/ie/plugin/bho/browser_helper_object.h b/ceee/ie/plugin/bho/browser_helper_object.h
index bf4afb9..a216556 100644
--- a/ceee/ie/plugin/bho/browser_helper_object.h
+++ b/ceee/ie/plugin/bho/browser_helper_object.h
@@ -11,6 +11,7 @@
 #include <mshtml.h>  // Needed for exdisp.h
 #include <exdisp.h>
 #include <exdispid.h>
+#include <deque>
 #include <map>
 #include <set>
 #include <string>
@@ -39,6 +40,7 @@ class ATL_NO_VTABLE BrowserHelperObject
       public IDispEventSimpleImpl<0,
                                   BrowserHelperObject,
                                   &DIID_DWebBrowserEvents2>,
+      public IPersistImpl<BrowserHelperObject>,
       public IFrameEventHandlerHost,
       public IExtensionPortMessagingProvider,
       public IChromeFrameHostEvents,
@@ -50,6 +52,7 @@ class ATL_NO_VTABLE BrowserHelperObject
 
   BEGIN_COM_MAP(BrowserHelperObject)
     COM_INTERFACE_ENTRY(IObjectWithSite)
+    COM_INTERFACE_ENTRY(IPersist)
     COM_INTERFACE_ENTRY_IID(IID_IFrameEventHandlerHost, IFrameEventHandlerHost)
   END_COM_MAP()
 
diff --git a/ceee/ie/plugin/toolband/tool_band.cc b/ceee/ie/plugin/toolband/tool_band.cc
index eee6488..89e8be1 100644
--- a/ceee/ie/plugin/toolband/tool_band.cc
+++ b/ceee/ie/plugin/toolband/tool_band.cc
@@ -18,6 +18,7 @@
 #include "ceee/common/windows_constants.h"
 #include "ceee/ie/common/extension_manifest.h"
 #include "ceee/ie/common/ceee_module_util.h"
+#include "ceee/ie/plugin/bho/browser_helper_object.h"
 #include "ceee/ie/plugin/bho/tool_band_visibility.h"
 #include "chrome/common/automation_constants.h"
 #include "chrome/common/chrome_switches.h"
@@ -433,7 +434,7 @@ STDMETHODIMP_(void) ToolBand::OnCfReadyStateChanged(LONG state) {
 }
 
 STDMETHODIMP_(void) ToolBand::OnCfMessage(IDispatch* event) {
-  VARIANT origin = {VT_NULL};
+  CComVariant origin;
   HRESULT hr = event->Invoke(ComMessageEvent::DISPID_MESSAGE_EVENT_ORIGIN,
       IID_NULL, 0, DISPATCH_PROPERTYGET, 0, &origin, 0, 0);
   if (FAILED(hr) || origin.vt != VT_BSTR) {
@@ -441,7 +442,7 @@ STDMETHODIMP_(void) ToolBand::OnCfMessage(IDispatch* event) {
     return;
   }
 
-  VARIANT data_bstr = {VT_NULL};
+  CComVariant data_bstr;
   hr = event->Invoke(ComMessageEvent::DISPID_MESSAGE_EVENT_DATA,
       IID_NULL, 0, DISPATCH_PROPERTYGET, 0, &data_bstr, 0, 0);
   if (FAILED(hr) || data_bstr.vt != VT_BSTR) {
@@ -515,6 +516,7 @@ STDMETHODIMP_(void) ToolBand::OnCfExtensionReady(BSTR path, int response) {
 STDMETHODIMP_(void) ToolBand::OnCfGetEnabledExtensionsComplete(
     SAFEARRAY* extension_directories) {
   CComSafeArray<BSTR> directories;
+  EnsureBhoIsAvailable();  // We will require a working bho to do anything.
   directories.Attach(extension_directories);  // MUST DETACH BEFORE RETURNING
 
   // TODO(joi@chromium.org) Handle multiple extensions.
@@ -571,10 +573,10 @@ STDMETHODIMP_(void) ToolBand::OnIeNavigateComplete2(IDispatch* dispatch,
                                                     VARIANT* url) {
   // The flag is cleared on navigation complete since at this point we are
   // certain the process of placing the toolband has been completed.
-  // Doing it in GetBandInfo proved premature as many queries are expeced.
+  // Doing it in GetBandInfo proved premature as many queries are expected.
   ClearForceOwnLineFlag();
 
-  // We need to clear that flag just once. Now that's done, unadvise.
+  // Now that deferred initializations are done, unadvise.
   DCHECK(web_browser_ != NULL);
   if (web_browser_ && listening_to_browser_events_) {
     HostingBrowserEvents::DispEventUnadvise(web_browser_,
@@ -632,3 +634,64 @@ void ToolBand::ClearForceOwnLineFlag() {
     ceee_module_util::SetOptionToolbandForceReposition(false);
   }
 }
+
+HRESULT ToolBand::EnsureBhoIsAvailable() {
+  if (m_spUnkSite == NULL || web_browser_ == NULL) {
+    NOTREACHED() << "Invalid client site";
+    return E_FAIL;
+  }
+
+  CComBSTR bho_class_id_bstr(CLSID_BrowserHelperObject);
+  CComVariant existing_bho;
+  HRESULT hr = web_browser_->GetProperty(bho_class_id_bstr, &existing_bho);
+
+  if (FAILED(hr)) {
+    LOG(ERROR) << "Failed to retrieve BHO through GetProperty call: 0x" <<
+        std::hex << hr;
+    return hr;
+  }
+
+  if (existing_bho.vt != VT_EMPTY) {
+    DCHECK(existing_bho.vt == VT_UNKNOWN && existing_bho.punkVal != NULL);
+#ifndef NDEBUG
+    if (existing_bho.vt == VT_UNKNOWN && existing_bho.punkVal != NULL) {
+      // This is a sanity / assumption check regarding what we should regard
+      // as a valid BHO.
+      ScopedComPtr<IPersist> bho_iid_access;
+      HRESULT hr2 = bho_iid_access.QueryFrom(existing_bho.punkVal);
+      DCHECK(SUCCEEDED(hr2) && bho_iid_access.get() != NULL);
+      if (SUCCEEDED(hr2) && bho_iid_access.get() != NULL) {
+        CLSID calling_card = {};
+        hr2 = bho_iid_access->GetClassID(&calling_card);
+        DCHECK(SUCCEEDED(hr2) && calling_card == CLSID_BrowserHelperObject);
+      }
+    }
+#endif
+    DVLOG(1) << "BHO already loaded";
+    return S_OK;
+  }
+
+  ScopedComPtr<IObjectWithSite> bho;
+  hr = CreateBhoInstance(bho.Receive());
+
+  if (FAILED(hr)) {
+    NOTREACHED() << "Failed to create CEEE BHO." << com::LogHr(hr);
+    return hr;
+  }
+
+  hr = bho->SetSite(web_browser_);
+  if (FAILED(hr)) {
+    NOTREACHED() << "CEEE BHO SetSite failed.:" << com::LogHr(hr);
+    return hr;
+  }
+
+  hr = web_browser_->PutProperty(bho_class_id_bstr, CComVariant(bho));
+  LOG_IF(ERROR, FAILED(hr)) << "Assigning BHO to the web browser failed.";
+  LOG_IF(INFO, SUCCEEDED(hr)) << "CEEE BHO has been created by the toolband.";
+  return hr;
+}
+
+HRESULT ToolBand::CreateBhoInstance(IObjectWithSite** new_bho_instance) {
+  DCHECK(new_bho_instance != NULL && *new_bho_instance == NULL);
+  return BrowserHelperObject::CreateInstance(new_bho_instance);
+}
diff --git a/ceee/ie/plugin/toolband/tool_band.h b/ceee/ie/plugin/toolband/tool_band.h
index e66993f..1fbe73f 100644
--- a/ceee/ie/plugin/toolband/tool_band.h
+++ b/ceee/ie/plugin/toolband/tool_band.h
@@ -171,6 +171,12 @@ class ATL_NO_VTABLE ToolBand : public CComObjectRootEx<CComSingleThreadModel>,
   void OnSize(UINT type, CSize size);
   // @}
 
+  // Toolband requires a specific BHO to work. Normally, IE will create BHO
+  // about the same time as the toolband. However, in some scenarios
+  // (re-enabling the gadget) BHOs for the current pages won't be created,
+  // even though the bar will.
+  HRESULT EnsureBhoIsAvailable();
+
  private:
   // Initializes the toolband to the given site.
   // Called from SetSite.
@@ -195,6 +201,9 @@ class ATL_NO_VTABLE ToolBand : public CComObjectRootEx<CComSingleThreadModel>,
   bool ShouldForceOwnLine();
   void ClearForceOwnLineFlag();
 
+  // A test seam for BHO instantiation test.
+  virtual HRESULT CreateBhoInstance(IObjectWithSite** new_bho_instance);
+
   // The web browser that initialized this toolband.
   CComPtr<IWebBrowser2> web_browser_;
   // Our parent window, yielded by our site's IOleWindow.
diff --git a/ceee/ie/plugin/toolband/tool_band_unittest.cc b/ceee/ie/plugin/toolband/tool_band_unittest.cc
index cc8c3ca..6dc184a 100644
--- a/ceee/ie/plugin/toolband/tool_band_unittest.cc
+++ b/ceee/ie/plugin/toolband/tool_band_unittest.cc
@@ -9,7 +9,9 @@
 #include <shlguid.h>
 
 #include "ceee/common/initializing_coclass.h"
+#include "ceee/common/com_utils.h"
 #include "ceee/ie/common/mock_ceee_module_util.h"
+#include "ceee/ie/plugin/bho/browser_helper_object.h"
 #include "ceee/ie/testing/mock_browser_and_friends.h"
 #include "ceee/testing/utils/dispex_mocks.h"
 #include "ceee/testing/utils/instance_count_mixin.h"
@@ -21,28 +23,69 @@
 
 namespace {
 
+using testing::_;
 using testing::GetConnectionCount;
 using testing::InstanceCountMixin;
 using testing::MockDispatchEx;
 using testing::Return;
+using testing::StrEq;
 using testing::StrictMock;
 using testing::TestBrowser;
 using testing::TestBrowserSite;
+using testing::WithArgs;
 
-// Makes ToolBand testable - circumvents InitializeAndShowWindow.
+class MockCeeeBho
+  : public CComObjectRootEx<CComSingleThreadModel>,
+    public InitializingCoClass<MockCeeeBho>,
+    public IObjectWithSiteImpl<MockCeeeBho>,
+    public IPersistImpl<MockCeeeBho> {
+ public:
+  BEGIN_COM_MAP(MockCeeeBho)
+    COM_INTERFACE_ENTRY(IObjectWithSite)
+    COM_INTERFACE_ENTRY(IPersist)
+  END_COM_MAP()
+
+  static const CLSID& WINAPI GetObjectCLSID() {
+    // Required to make IPersistImpl work.
+    return BrowserHelperObject::GetObjectCLSID();
+  }
+
+  HRESULT Initialize(MockCeeeBho** self) {
+    *self = this;
+    return S_OK;
+  }
+};
+
+// Makes ToolBand testable - circumvents InitializeAndShowWindow and exposes
+// some otherwise protected functionality.
 class TestingToolBand
     : public ToolBand,
       public InstanceCountMixin<TestingToolBand>,
       public InitializingCoClass<TestingToolBand> {
  public:
   HRESULT Initialize(TestingToolBand** self) {
+    bho_counter_ = 0;
     *self = this;
     return S_OK;
   }
+
+  HRESULT CallEnsureBhoIsAvailable() {
+    return EnsureBhoIsAvailable();
+  }
+
+  int bho_counter_;
  private:
   virtual HRESULT InitializeAndShowWindow(IUnknown* site) {
     return S_OK;  // This aspect is not tested.
   }
+
+  HRESULT CreateBhoInstance(IObjectWithSite** new_bho_instance) {
+    DCHECK(new_bho_instance != NULL && *new_bho_instance == NULL);
+    MockCeeeBho* correctly_typed_bho = NULL;
+    ++bho_counter_;
+    return MockCeeeBho::CreateInitialized(&correctly_typed_bho,
+                                          new_bho_instance);
+  }
 };
 
 class ToolBandTest: public testing::Test {
@@ -360,4 +403,87 @@ TEST_F(ToolBandTest, NewInstallationTriggersLineBreak) {
   ASSERT_HRESULT_SUCCEEDED(tool_band_with_site_->SetSite(NULL));
 }
 
+class BhoInToolBandTest : public ToolBandTest {
+ public:
+  virtual void SetUp() {
+    ToolBandTest::SetUp();
+
+    // In addition to whatever all other toolband tests required, bho will
+    // need a properly constructed site.
+    CreateSite();
+    CreateBrowser();
+
+    EXPECT_CALL(ceee_module_utils_, GetOptionToolbandForceReposition())
+        .WillOnce(Return(false));
+
+    ASSERT_HRESULT_SUCCEEDED(tool_band_with_site_->SetSite(site_keeper_));
+    ASSERT_TRUE(com::GuidToString(CLSID_BrowserHelperObject,
+                                  &bho_class_id_text_));
+  }
+
+  std::wstring bho_class_id_text_;
+};
+
+ACTION_P(SetArg1Variant, local_data_out) {
+  return ::VariantCopy(arg1, local_data_out);
+}
+
+ACTION_P(GetArg1Variant, local_data_in) {
+  return local_data_in->Copy(&arg1);
+}
+
+TEST_F(BhoInToolBandTest, EnsureBhoCreatedWhenNeeded) {
+  // For actual tests, use an instance of mock BHO.
+  MockCeeeBho* mocked_bho;
+  CComPtr<IObjectWithSite> mocked_bho_with_site;
+  ASSERT_HRESULT_SUCCEEDED(
+      MockCeeeBho::CreateInitialized(&mocked_bho, &mocked_bho_with_site));
+
+  // If there already is a bho of this code, do not create it.
+  CComVariant variant_with_bho(mocked_bho_with_site);
+  EXPECT_CALL(*browser_, GetProperty(StrEq(bho_class_id_text_), _)).
+      WillOnce(SetArg1Variant(&variant_with_bho));
+  ASSERT_HRESULT_SUCCEEDED(tool_band_->CallEnsureBhoIsAvailable());
+  ASSERT_TRUE(mocked_bho->m_spUnkSite == NULL);
+
+  // If there was no BHO, it will get created. Should be of known type
+  // and contain the appropriate site.
+  CComVariant bho_dropbox;
+  // Empty dropbox will trigger bho construction process.
+  EXPECT_CALL(*browser_, GetProperty(StrEq(bho_class_id_text_), _)).
+      WillOnce(SetArg1Variant(&bho_dropbox));
+  // New bho should be dropped into the dropbox.
+  EXPECT_CALL(*browser_, PutProperty(StrEq(bho_class_id_text_), _)).
+      WillOnce(GetArg1Variant(&bho_dropbox));
+  ASSERT_HRESULT_SUCCEEDED(tool_band_->CallEnsureBhoIsAvailable());
+  ASSERT_EQ(bho_dropbox.vt, VT_UNKNOWN);
+  ASSERT_TRUE(bho_dropbox.punkVal != NULL);
+
+  CComPtr<IUnknown> retrieved_bho_raw(bho_dropbox.punkVal);
+  CComPtr<IObjectWithSite> retrieved_bho;
+  ASSERT_HRESULT_SUCCEEDED(retrieved_bho_raw.QueryInterface(&retrieved_bho));
+
+  CComPtr<IUnknown> applied_site;
+  ASSERT_HRESULT_SUCCEEDED(retrieved_bho->GetSite(IID_IUnknown,
+      reinterpret_cast<void**>(&applied_site)));
+
+  ASSERT_TRUE(applied_site.IsEqualObject(browser_keeper_));
+}
+
+TEST_F(BhoInToolBandTest, BhoCreationErrorHandling) {
+  testing::LogDisabler no_dchecks;
+
+  // A failed call to get the property causes error.
+  EXPECT_CALL(*browser_, GetProperty(StrEq(bho_class_id_text_), _)).
+      WillOnce(Return(E_FAIL));
+  ASSERT_HRESULT_FAILED(tool_band_->CallEnsureBhoIsAvailable());
+
+  // Anything (whatever it is) in the property will not cause creating bho.
+  CComVariant weird_content(browser_);
+  EXPECT_CALL(*browser_, GetProperty(StrEq(bho_class_id_text_), _)).
+      WillOnce(SetArg1Variant(&weird_content));
+  tool_band_->CallEnsureBhoIsAvailable();
+  ASSERT_EQ(tool_band_->bho_counter_, 0);
+}
+
 }  // namespace