Fixed unittests broken by misplaced BrokerRpcClient::Connect call.

BUG=none
TEST=none

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

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

8 files changed, 56 insertions, 15 deletions

diff --git a/ceee/ie/plugin/bho/browser_helper_object.cc b/ceee/ie/plugin/bho/browser_helper_object.cc
index 8f5bc41..9d0e5a0a5 100644
--- a/ceee/ie/plugin/bho/browser_helper_object.cc
+++ b/ceee/ie/plugin/bho/browser_helper_object.cc
@@ -119,11 +119,8 @@ HRESULT BrowserHelperObject::FinalConstruct() {
     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;
   }
+  return S_OK;
 }
 
 void BrowserHelperObject::FinalRelease() {
@@ -392,6 +389,9 @@ HRESULT BrowserHelperObject::Initialize(IUnknown* site) {
     return hr;
   }
 
+  // Do before HttpNegotiatePatch::Initialize.
+  CookieAccountant::GetInstance()->Initialize();
+
   // Patch IHttpNegotiate for user-agent and cookie functionality.
   HttpNegotiatePatch::Initialize();
 
diff --git a/ceee/ie/plugin/bho/cookie_accountant.cc b/ceee/ie/plugin/bho/cookie_accountant.cc
index 3d0f3f1..1445b97 100644
--- a/ceee/ie/plugin/bho/cookie_accountant.cc
+++ b/ceee/ie/plugin/bho/cookie_accountant.cc
@@ -226,13 +226,22 @@ void CookieAccountant::RecordHttpResponseCookies(
   }
 }
 
-void CookieAccountant::PatchWininetFunctions() {
+void CookieAccountant::ConnectBroker() {
+  if (!broker_rpc_client_.is_connected()) {
+    HRESULT hr = broker_rpc_client_.Connect(true);
+    DCHECK(SUCCEEDED(hr));
+  }
+}
+
+void CookieAccountant::Initialize() {
   {
     AutoLock lock(lock_);
-    if (patching_wininet_functions_)
+    if (initializing_)
       return;
-    patching_wininet_functions_ = true;
+    initializing_ = true;
   }
+  ConnectBroker();
+
   if (!internet_set_cookie_ex_a_patch_.is_patched()) {
     DWORD error = internet_set_cookie_ex_a_patch_.Patch(
         kMsHtmlModuleName, kWinInetModuleName,
@@ -249,6 +258,6 @@ void CookieAccountant::PatchWininetFunctions() {
          internet_set_cookie_ex_w_patch_.is_patched());
   {
     AutoLock lock(lock_);
-    patching_wininet_functions_ = false;
+    initializing_ = false;
   }
 }
diff --git a/ceee/ie/plugin/bho/cookie_accountant.h b/ceee/ie/plugin/bho/cookie_accountant.h
index a2b9de3..23111de 100644
--- a/ceee/ie/plugin/bho/cookie_accountant.h
+++ b/ceee/ie/plugin/bho/cookie_accountant.h
@@ -26,7 +26,7 @@
 class CookieAccountant {
  public:
   // Patch cookie-related functions to observe IE session cookies.
-  void PatchWininetFunctions();
+  void Initialize();
 
   // Record Set-Cookie changes coming from the HTTP response headers.
   void RecordHttpResponseCookies(
@@ -56,9 +56,7 @@ class CookieAccountant {
   CookieAccountant()
       : broker_rpc_client_(true),
         cookie_events_funnel_(&broker_rpc_client_),
-        patching_wininet_functions_(false) {
-    HRESULT hr = broker_rpc_client_.Connect(true);
-    DCHECK(SUCCEEDED(hr));
+        initializing_(false) {
   }
 
   virtual ~CookieAccountant();
@@ -74,6 +72,9 @@ class CookieAccountant {
     return cookie_events_funnel_;
   }
 
+  // Connects to broker.
+  virtual void ConnectBroker();
+
   // Function patches that allow us to intercept scripted cookie changes.
   app::win::IATPatchFunction internet_set_cookie_ex_a_patch_;
   app::win::IATPatchFunction internet_set_cookie_ex_w_patch_;
@@ -82,7 +83,7 @@ class CookieAccountant {
   // We use this boolean instead of a simple lock so that threads that lose
   // the race will return immediately instead of blocking on the lock.
   // Protected by CookieAccountant::lock_.
-  bool patching_wininet_functions_;
+  bool initializing_;
 
   // A lock that protects access to the function patches.
   Lock lock_;
diff --git a/ceee/ie/plugin/bho/cookie_accountant_unittest.cc b/ceee/ie/plugin/bho/cookie_accountant_unittest.cc
index 6bd00ec..e463bb2 100644
--- a/ceee/ie/plugin/bho/cookie_accountant_unittest.cc
+++ b/ceee/ie/plugin/bho/cookie_accountant_unittest.cc
@@ -69,6 +69,10 @@ class MockCookieAccountant : public CookieAccountant {
     return mock_cookie_events_funnel_;
   }
 
+  virtual void ConnectBroker() {
+    return;
+  }
+
   static void set_singleton_instance(CookieAccountant* instance) {
     singleton_instance_ = instance;
   }
diff --git a/ceee/ie/plugin/bho/web_progress_notifier_unittest.cc b/ceee/ie/plugin/bho/web_progress_notifier_unittest.cc
index 662254a..28eb3fd 100644
--- a/ceee/ie/plugin/bho/web_progress_notifier_unittest.cc
+++ b/ceee/ie/plugin/bho/web_progress_notifier_unittest.cc
@@ -140,6 +140,18 @@ class TestWebProgressNotifier : public WebProgressNotifier {
     return previous_travel_log_info_;
   }
 
+  virtual WebRequestNotifier* webrequest_notifier() {
+    return &web_request_notifier_;
+  }
+
+  class TestingWebRequestNotifier : public WebRequestNotifier {
+   public:
+    virtual bool ConnectBroker() {
+      return true;
+    }
+  };
+
+  TestingWebRequestNotifier web_request_notifier_;
   StrictMock<testing::MockWebNavigationEventsFunnel>
       mock_webnavigation_events_funnel_;
   bool mock_is_forward_back_;
diff --git a/ceee/ie/plugin/bho/webrequest_notifier.cc b/ceee/ie/plugin/bho/webrequest_notifier.cc
index c0487a3..beb4c60 100644
--- a/ceee/ie/plugin/bho/webrequest_notifier.cc
+++ b/ceee/ie/plugin/bho/webrequest_notifier.cc
@@ -34,14 +34,19 @@ WebRequestNotifier::WebRequestNotifier()
           initialize_state_(NOT_INITIALIZED),
           broker_rpc_client_(true),
           webrequest_events_funnel_(&broker_rpc_client_) {
-  HRESULT hr = broker_rpc_client_.Connect(true);
-  DCHECK(SUCCEEDED(hr));
 }
 
 WebRequestNotifier::~WebRequestNotifier() {
   DCHECK_EQ(start_count_, 0);
 }
 
+bool WebRequestNotifier::ConnectBroker() {
+  if (broker_rpc_client_.is_connected())
+    return true;
+  HRESULT hr = broker_rpc_client_.Connect(true);
+  DCHECK(SUCCEEDED(hr));
+  return SUCCEEDED(hr);
+}
 bool WebRequestNotifier::RequestToStart() {
   {
     CComCritSecLock<CComAutoCriticalSection> lock(critical_section_);
@@ -54,6 +59,9 @@ bool WebRequestNotifier::RequestToStart() {
 
   bool success = false;
   do {
+    if (!ConnectBroker())
+      break;
+
     // We are not going to unpatch any of the patched WinINet functions or the
     // status callback function. Instead, we pin our DLL in memory so that all
     // the patched functions can be accessed until the process goes away.
diff --git a/ceee/ie/plugin/bho/webrequest_notifier.h b/ceee/ie/plugin/bho/webrequest_notifier.h
index d7c1227..3a02616 100644
--- a/ceee/ie/plugin/bho/webrequest_notifier.h
+++ b/ceee/ie/plugin/bho/webrequest_notifier.h
@@ -145,6 +145,9 @@ class WebRequestNotifier {
     return webrequest_events_funnel_;
   }
 
+  // Connects to broker.
+  virtual bool ConnectBroker();
+
   // Gets called before calling InternetSetStatusCallback.
   // @param internet The handle for which the callback is set.
   // @param callback The real callback function.
diff --git a/ceee/ie/plugin/bho/webrequest_notifier_unittest.cc b/ceee/ie/plugin/bho/webrequest_notifier_unittest.cc
index 81ee4d0..8f73219 100644
--- a/ceee/ie/plugin/bho/webrequest_notifier_unittest.cc
+++ b/ceee/ie/plugin/bho/webrequest_notifier_unittest.cc
@@ -32,6 +32,10 @@ class TestWebRequestNotifier : public WebRequestNotifier {
     return mock_webrequest_events_funnel_;
   }
 
+  virtual bool ConnectBroker() {
+    return true;
+  }
+
   virtual bool QueryHttpInfoString(HINTERNET /*request*/,
                                    DWORD info_flag,
                                    std::wstring* value) {