Use the RPC channel to set the ID <-> handle mapping.

As described in more details in the bug description, if we use COM for these and RPC for the other events, then they could arrive out of order.

BUG=65316
TEST=Create as many tabs as quickly as possible and watch the logs (sawbuck) for errors.
Review URL: http://codereview.chromium.org/5647001

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

6 files changed, 108 insertions, 29 deletions

diff --git a/ceee/ie/broker/tab_api_module.cc b/ceee/ie/broker/tab_api_module.cc
index b50b5ff..420007e 100644
--- a/ceee/ie/broker/tab_api_module.cc
+++ b/ceee/ie/broker/tab_api_module.cc
@@ -126,6 +126,53 @@ bool CeeeUnmapTabEventHandler(const std::string& input_args,
   return false;
 }
 
+bool GetIdAndHandleFromArgs(const std::string& input_args, int* id,
+                            HWND* handle) {
+  DCHECK(id != NULL);
+  DCHECK(handle != NULL);
+  scoped_ptr<ListValue> input_list;
+  *id = kInvalidChromeSessionId;
+  if (!api_module_util::GetListAndIntegerValue(input_args, &input_list, id) ||
+      *id == kInvalidChromeSessionId) {
+    NOTREACHED() << "An invalid argument was passed to GetIdAndHandleFromArgs";
+    return false;
+  }
+
+  int tab_handle = 0;
+  if (!input_list->GetInteger(1, &tab_handle) || tab_handle == 0) {
+    NOTREACHED() << "An invalid argument was passed to GetIdAndHandleFromArgs";
+    return false;
+  }
+  *handle = reinterpret_cast<HWND>(tab_handle);
+  return true;
+}
+
+bool CeeeMapTabIdToHandle(const std::string& input_args,
+                          std::string* converted_args,
+                          ApiDispatcher* dispatcher) {
+  int tab_id = kInvalidChromeSessionId;
+  HWND tab_handle = NULL;
+  if (GetIdAndHandleFromArgs(input_args, &tab_id, &tab_handle)) {
+    Singleton<ExecutorsManager, ExecutorsManager::SingletonTraits>::get()->
+        SetTabIdForHandle(tab_id, tab_handle);
+    return true;
+  }
+  return false;
+}
+
+bool CeeeMapToolbandIdToHandle(const std::string& input_args,
+                               std::string* converted_args,
+                               ApiDispatcher* dispatcher) {
+  int toolband_id = kInvalidChromeSessionId;
+  HWND tab_handle = NULL;
+  if (GetIdAndHandleFromArgs(input_args, &toolband_id, &tab_handle)) {
+    Singleton<ExecutorsManager, ExecutorsManager::SingletonTraits>::get()->
+        SetTabToolBandIdForHandle(toolband_id, tab_handle);
+    return true;
+  }
+  return false;
+}
+
 }  // namespace
 
 void RegisterInvocations(ApiDispatcher* dispatcher) {
@@ -137,6 +184,10 @@ void RegisterInvocations(ApiDispatcher* dispatcher) {
   // Registers our private events.
   dispatcher->RegisterPermanentEventHandler(
       ceee_event_names::kCeeeOnTabUnmapped, CeeeUnmapTabEventHandler);
+  dispatcher->RegisterPermanentEventHandler(
+      ceee_event_names::kCeeeMapTabIdToHandle, CeeeMapTabIdToHandle);
+  dispatcher->RegisterPermanentEventHandler(
+      ceee_event_names::kCeeeMapToolbandIdToHandle, CeeeMapToolbandIdToHandle);
 
   // And now register the permanent event handlers.
   dispatcher->RegisterPermanentEventHandler(ext_event_names::kOnTabCreated,
diff --git a/ceee/ie/common/constants.cc b/ceee/ie/common/constants.cc
index 64a1d52..b25a1b5 100644
--- a/ceee/ie/common/constants.cc
+++ b/ceee/ie/common/constants.cc
@@ -8,4 +8,6 @@
 
 namespace ceee_event_names {
   const char kCeeeOnTabUnmapped[] = "ceee.OnTabUnmapped";
+  const char kCeeeMapTabIdToHandle[] = "ceee.MapTabIdToHandle";
+  const char kCeeeMapToolbandIdToHandle[] = "ceee.MapToolbandIdToHandle";
 }
diff --git a/ceee/ie/common/constants.h b/ceee/ie/common/constants.h
index 600a589..1f75df8 100644
--- a/ceee/ie/common/constants.h
+++ b/ceee/ie/common/constants.h
@@ -8,6 +8,8 @@
 namespace ceee_event_names {
   // Private messages used by the ApiDispatcher and the funnels.
   extern const char kCeeeOnTabUnmapped[];
+  extern const char kCeeeMapTabIdToHandle[];
+  extern const char kCeeeMapToolbandIdToHandle[];
 }
 
 #endif  // CEEE_IE_COMMON_CONSTANTS_H_
diff --git a/ceee/ie/plugin/bho/browser_helper_object.cc b/ceee/ie/plugin/bho/browser_helper_object.cc
index ae4222d..24fbaf8 100644
--- a/ceee/ie/plugin/bho/browser_helper_object.cc
+++ b/ceee/ie/plugin/bho/browser_helper_object.cc
@@ -12,6 +12,7 @@
 
 #include "base/debug/trace_event.h"
 #include "base/json/json_reader.h"
+#include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "base/tuple.h"
@@ -20,6 +21,7 @@
 #include "ceee/common/window_utils.h"
 #include "ceee/common/windows_constants.h"
 #include "ceee/ie/broker/tab_api_module.h"
+#include "ceee/ie/common/constants.h"
 #include "ceee/ie/common/extension_manifest.h"
 #include "ceee/ie/common/ie_util.h"
 #include "ceee/ie/common/metrics_util.h"
@@ -327,24 +329,6 @@ HRESULT BrowserHelperObject::UnregisterExecutor(DWORD thread_id) {
   return registrar->UnregisterExecutor(thread_id);
 }
 
-HRESULT BrowserHelperObject::SetTabToolBandIdForHandle(int tool_band_id,
-                                                       long tab_handle) {
-  base::win::ScopedComPtr<ICeeeBrokerRegistrar> registrar;
-  HRESULT hr = GetBrokerRegistrar(registrar.Receive());
-  if (FAILED(hr))
-    return hr;
-  return registrar->SetTabToolBandIdForHandle(tool_band_id, tab_handle);
-}
-
-HRESULT BrowserHelperObject::SetTabIdForHandle(int tool_band_id,
-                                               long tab_handle) {
-  base::win::ScopedComPtr<ICeeeBrokerRegistrar> registrar;
-  HRESULT hr = GetBrokerRegistrar(registrar.Receive());
-  if (FAILED(hr))
-    return hr;
-  return registrar->SetTabIdForHandle(tool_band_id, tab_handle);
-}
-
 HRESULT BrowserHelperObject::Initialize(IUnknown* site) {
   TRACE_EVENT_INSTANT("ceee.bho.initialize", this, "");
   mu::ScopedTimer metrics_timer("ceee/BHO.Initialize", &broker_rpc());
@@ -646,15 +630,14 @@ bool BrowserHelperObject::EnsureTabId() {
     return false;
   }
 
-  CeeeWindowHandle handle = reinterpret_cast<CeeeWindowHandle>(tab_window_);
-  hr = SetTabIdForHandle(tab_id_, handle);
+  hr = FireMapTabIdToHandle();
   if (FAILED(hr)) {
     DCHECK(SUCCEEDED(hr)) << "An error occured when setting the tab_id: " <<
         com::LogHr(hr);
     tab_id_ = kInvalidChromeSessionId;
     return false;
   }
-  VLOG(2) << "TabId(" << tab_id_ << ") set for Handle(" << handle << ")";
+  VLOG(2) << "TabId(" << tab_id_ << ") set for Handle(" << tab_window_ << ")";
 
   // Call back all the events we deferred. In order, please.
   while (!deferred_events_call_.empty()) {
@@ -723,6 +706,34 @@ HRESULT BrowserHelperObject::CreateChromeFrameHost() {
                                                chrome_frame_host_.Receive());
 }
 
+HRESULT BrowserHelperObject::SendIdMappingToBroker(const char* event_name,
+                                                   int id) {
+  // Event arguments for FireEvent need to be stored in a list.
+  VLOG(1) << "SendIdMappingToBroker(" << event_name << ", " << id << ")";
+  DCHECK(id != kInvalidChromeSessionId);
+  DCHECK(tab_window_ != NULL);
+
+  ListValue mapping_args;
+  mapping_args.Append(Value::CreateIntegerValue(id));
+  mapping_args.Append(Value::CreateIntegerValue(
+      reinterpret_cast<int>(tab_window_)));
+
+  std::string event_args_str;
+  base::JSONWriter::Write(&mapping_args, false, &event_args_str);
+  // We go directly to Impl version since we know we have Ensured Id...
+  return SendEventToBrokerImpl(event_name, event_args_str);
+}
+
+HRESULT BrowserHelperObject::FireMapTabIdToHandle() {
+  return SendIdMappingToBroker(ceee_event_names::kCeeeMapTabIdToHandle,
+                               tab_id_);
+}
+
+HRESULT BrowserHelperObject::FireMapToolbandIdToHandle(int toolband_id) {
+  return SendIdMappingToBroker(ceee_event_names::kCeeeMapToolbandIdToHandle,
+                               toolband_id);
+}
+
 void BrowserHelperObject::FireOnCreatedEvent(BSTR url) {
   DCHECK(url != NULL);
   DCHECK(tab_window_ != NULL);
@@ -1575,15 +1586,14 @@ void BrowserHelperObject::UnregisterSink(Sink* sink) {
 }
 
 STDMETHODIMP BrowserHelperObject::SetToolBandSessionId(long session_id) {
-  CeeeWindowHandle handle = reinterpret_cast<CeeeWindowHandle>(tab_window_);
-  HRESULT hr = SetTabToolBandIdForHandle(session_id, handle);
+  HRESULT hr = FireMapToolbandIdToHandle(session_id);
   if (FAILED(hr)) {
     DCHECK(SUCCEEDED(hr)) << "An error occured when setting the toolband " <<
         "tab ID: " << com::LogHr(hr);
     return hr;
   }
-  VLOG(2) << "ToolBandTabId(" << session_id << ") set for Handle(" << handle <<
-      ")";
+  VLOG(2) << "ToolBandTabId(" << session_id << ") set for Handle(" <<
+      tab_window_ << ")";
   return hr;
 }
 
diff --git a/ceee/ie/plugin/bho/browser_helper_object.h b/ceee/ie/plugin/bho/browser_helper_object.h
index 32be1b6..8a699c3 100644
--- a/ceee/ie/plugin/bho/browser_helper_object.h
+++ b/ceee/ie/plugin/bho/browser_helper_object.h
@@ -262,6 +262,13 @@ class ATL_NO_VTABLE BrowserHelperObject
   // Accessor so that we can mock it in unit tests.
   virtual BrokerRpcClient& broker_rpc() { return broker_rpc_; }
 
+  // Fires the private message to map a tab id to its associated tab handle.
+  virtual HRESULT FireMapTabIdToHandle();
+
+  // Fires the private message to map a tool band id to its associated tab
+  // handle.
+  virtual HRESULT FireMapToolbandIdToHandle(int toolband_id);
+
   // Fires the tab.onCreated event via the tab event funnel.
   virtual void FireOnCreatedEvent(BSTR url);
 
@@ -436,6 +443,10 @@ class ATL_NO_VTABLE BrowserHelperObject
     DISALLOW_COPY_AND_ASSIGN(BrokerEventQueue);
   };
 
+  // This synchronously send the association between the given ID and our
+  // current tab handle using the given event name.
+  HRESULT SendIdMappingToBroker(const char* event_name, int id);
+
   // This either calls its Impl or queues the call for later, when we have a
   // tab_id.
   HRESULT SendEventToBroker(const char* event_name, const char* event_args);
diff --git a/ceee/ie/plugin/bho/browser_helper_object_unittest.cc b/ceee/ie/plugin/bho/browser_helper_object_unittest.cc
index fada7d8..aae41cc 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/constants.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"
@@ -866,12 +867,14 @@ TEST_F(BrowserHelperObjectTest, SetToolBandSessionId) {
   CreateBrowser();
 
   ASSERT_HRESULT_SUCCEEDED(bho_with_site_->SetSite(site_keeper_));
-  EXPECT_CALL(*(bho_->broker_), SetTabToolBandIdForHandle(kGoodTabId,
-        kGoodTabHandle)).WillOnce(Return(S_OK));
+  EXPECT_CALL(bho_->mock_broker_rpc_client_, FireEvent(
+      StrEq(ceee_event_names::kCeeeMapToolbandIdToHandle), _)).
+          WillOnce(Return(S_OK));
   EXPECT_EQ(S_OK, bho_->SetToolBandSessionId(kGoodTabId));
 
-  EXPECT_CALL(*(bho_->broker_), SetTabToolBandIdForHandle(kGoodTabId,
-        kGoodTabHandle)).WillOnce(Return(E_FAIL));
+  EXPECT_CALL(bho_->mock_broker_rpc_client_, FireEvent(
+      StrEq(ceee_event_names::kCeeeMapToolbandIdToHandle), _)).
+          WillOnce(Return(E_FAIL));
   EXPECT_EQ(E_FAIL, bho_->SetToolBandSessionId(kGoodTabId));
 
   ExpectFireOnRemovedEvent(0);