Fail BHO construction if Toolband is disabled.

Construction of BHO will fail if the toolband is marked as 'invisible' in the registry. Unit tests were updated as required. 

Submitting for motek@google.com, original review at http://codereview.chromium.org/5322010/.

BUG=2840408
TEST=ie_unittest.exe 

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

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

4 files changed, 62 insertions, 21 deletions

diff --git a/ceee/ie/ie.gyp b/ceee/ie/ie.gyp
index 3a04651..8df221b 100644
--- a/ceee/ie/ie.gyp
+++ b/ceee/ie/ie.gyp
@@ -155,6 +155,7 @@
         '../../base/base.gyp:base',
         '../../testing/gmock.gyp:gmock',
         '../../testing/gtest.gyp:gtest',
+        '../testing/sidestep/sidestep.gyp:sidestep',
         '../testing/utils/test_utils.gyp:test_utils',
       ],
       'libraries': [
diff --git a/ceee/ie/plugin/bho/browser_helper_object.cc b/ceee/ie/plugin/bho/browser_helper_object.cc
index 09c40bc..47aa6ac 100644
--- a/ceee/ie/plugin/bho/browser_helper_object.cc
+++ b/ceee/ie/plugin/bho/browser_helper_object.cc
@@ -102,8 +102,6 @@ BrowserHelperObject::BrowserHelperObject()
       thread_id_(::GetCurrentThreadId()),
       full_tab_chrome_frame_(false) {
   TRACE_EVENT_BEGIN("ceee.bho", this, "");
-  // Only the first call to this function really does anything.
-  CookieAccountant::GetInstance()->PatchWininetFunctions();
 }
 
 BrowserHelperObject::~BrowserHelperObject() {
@@ -114,7 +112,16 @@ BrowserHelperObject::~BrowserHelperObject() {
 }
 
 HRESULT BrowserHelperObject::FinalConstruct() {
-  return S_OK;
+  if (ceee_module_util::GetOptionToolbandIsHidden()) {
+    // Patching wininet when BHO could not be created considered pointless.
+    LOG(INFO) <<
+        "Refused to instantiate the BHO when the visual component is hidden.";
+    return E_FAIL;
+  } else {
+    // Only the first call to this function really does anything.
+    CookieAccountant::GetInstance()->PatchWininetFunctions();
+    return S_OK;
+  }
 }
 
 void BrowserHelperObject::FinalRelease() {
diff --git a/ceee/ie/plugin/bho/browser_helper_object_unittest.cc b/ceee/ie/plugin/bho/browser_helper_object_unittest.cc
index 771ba15..ce7c5a5 100644
--- a/ceee/ie/plugin/bho/browser_helper_object_unittest.cc
+++ b/ceee/ie/plugin/bho/browser_helper_object_unittest.cc
@@ -9,6 +9,7 @@
 #include <shlguid.h>
 
 #include "ceee/common/initializing_coclass.h"
+#include "ceee/ie/common/mock_ceee_module_util.h"
 #include "ceee/ie/testing/mock_broker_and_friends.h"
 #include "ceee/ie/testing/mock_browser_and_friends.h"
 #include "ceee/ie/testing/mock_chrome_frame_host.h"
@@ -175,6 +176,10 @@ class BrowserHelperObjectTest: public testing::Test {
 
   virtual void SetUp() {
     // Create the instance to test.
+    // Force required registry state to prevent flakiness.
+    StrictMock<testing::MockCeeeModuleUtils> ceee_module_utils;
+    EXPECT_CALL(ceee_module_utils, GetOptionToolbandIsHidden())
+        .WillOnce(Return(false));
     ASSERT_HRESULT_SUCCEEDED(
         TestingBrowserHelperObject::CreateInitialized(&bho_, &bho_with_site_));
     bho_with_site_ = bho_;
@@ -200,14 +205,16 @@ class BrowserHelperObjectTest: public testing::Test {
   }
 
   virtual void TearDown() {
-    bho_->executor_ = NULL;
-    bho_->executor_keeper_.Release();
+    if (bho_ != NULL) {  // To match failure modes of SetUp.
+      bho_->executor_ = NULL;
+      bho_->executor_keeper_.Release();
 
-    bho_->broker_ = NULL;
-    bho_->broker_keeper_.Release();
+      bho_->broker_ = NULL;
+      bho_->broker_keeper_.Release();
 
-    bho_ = NULL;
-    bho_with_site_.Release();
+      bho_ = NULL;
+      bho_with_site_.Release();
+    }
 
     site_ = NULL;
     site_keeper_.Release();
@@ -871,4 +878,25 @@ TEST_F(BrowserHelperObjectTest, SetToolBandSessionId) {
   ASSERT_HRESULT_SUCCEEDED(bho_with_site_->SetSite(NULL));
 }
 
+TEST(BrowserHelperObjectLifeCycleTest, NoBhoForInvisibleToolband) {
+  StrictMock<testing::MockCeeeModuleUtils> ceee_module_utils;
+    EXPECT_CALL(ceee_module_utils, GetOptionToolbandIsHidden())
+        .WillOnce(Return(false));
+
+  TestingBrowserHelperObject* bho;
+  base::win::ScopedComPtr<IObjectWithSite> bho_with_site;
+  ASSERT_HRESULT_SUCCEEDED(
+      TestingBrowserHelperObject::CreateInitialized(&bho,
+          bho_with_site.Receive()));
+  bho_with_site.Release();
+  bho = NULL;
+
+  EXPECT_CALL(ceee_module_utils, GetOptionToolbandIsHidden())
+        .WillOnce(Return(true));
+  ASSERT_HRESULT_FAILED(
+      TestingBrowserHelperObject::CreateInitialized(&bho,
+          bho_with_site.Receive()));
+  ASSERT_EQ(0, testing::InstanceCountMixinBase::all_instance_count());
+}
+
 }  // namespace
diff --git a/ceee/ie/plugin/bho/mediumtest_browser_helper_object.cc b/ceee/ie/plugin/bho/mediumtest_browser_helper_object.cc
index ef355e3..217e04d 100644
--- a/ceee/ie/plugin/bho/mediumtest_browser_helper_object.cc
+++ b/ceee/ie/plugin/bho/mediumtest_browser_helper_object.cc
@@ -9,6 +9,7 @@
 #include <shlguid.h>
 #include "base/utf_string_conversions.h"
 #include "ceee/ie/common/ceee_module_util.h"
+#include "ceee/ie/common/mock_ceee_module_util.h"
 #include "ceee/ie/plugin/bho/browser_helper_object.h"
 #include "ceee/ie/testing/mediumtest_ie_common.h"
 #include "ceee/ie/testing/mock_broker_and_friends.h"
@@ -302,7 +303,9 @@ class BrowserHelperObjectTest: public ShellBrowserTestImpl<BrowserEventSink> {
 
   virtual void SetUp() {
     Super::SetUp();
-
+    StrictMock<testing::MockCeeeModuleUtils> ceee_module_utils;
+    EXPECT_CALL(ceee_module_utils, GetOptionToolbandIsHidden())
+        .WillRepeatedly(Return(false));
     // Never torn down as other threads in the test may need it after
     // teardown.
     ScriptHost::set_default_debug_application(&debug_app);
@@ -335,21 +338,23 @@ class BrowserHelperObjectTest: public ShellBrowserTestImpl<BrowserEventSink> {
   }
 
   virtual void TearDown() {
-    EXPECT_CALL(*bho_->mock_tab_events_funnel(), OnRemoved(_));
-    EXPECT_CALL(*bho_->mock_tab_events_funnel(), OnTabUnmapped(_, _));
-    ASSERT_HRESULT_SUCCEEDED(bho_keeper_->SetSite(NULL));
+    if (bho_ != NULL) {  // To match SetUp failure modes.
+      EXPECT_CALL(*bho_->mock_tab_events_funnel(), OnRemoved(_));
+      EXPECT_CALL(*bho_->mock_tab_events_funnel(), OnTabUnmapped(_, _));
+      ASSERT_HRESULT_SUCCEEDED(bho_keeper_->SetSite(NULL));
 
-    site_ = NULL;
-    site_keeper_.Release();
+      bho_->executor_ = NULL;
+      bho_->executor_keeper_.Release();
 
-    bho_->executor_ = NULL;
-    bho_->executor_keeper_.Release();
+      bho_->broker_ = NULL;
+      bho_->broker_keeper_.Release();
 
-    bho_->broker_ = NULL;
-    bho_->broker_keeper_.Release();
+      bho_ = NULL;
+      bho_keeper_.Release();
+    }
 
-    bho_ = NULL;
-    bho_keeper_.Release();
+    site_ = NULL;
+    site_keeper_.Release();
 
     Super::TearDown();
   }