diff options
24 files changed, 732 insertions, 138 deletions
diff --git a/ceee/ie/broker/api_dispatcher.cc b/ceee/ie/broker/api_dispatcher.cc index 0daad0c..e1ecec0 100644 --- a/ceee/ie/broker/api_dispatcher.cc +++ b/ceee/ie/broker/api_dispatcher.cc @@ -64,6 +64,12 @@ void ApiDispatcher::HandleApiRequest(BSTR message_text, BSTR* response) { DCHECK(false); return; } + + // The tab value is optional, and maybe NULL if the API call didn't originate + // from a tab context (e.g. the API call came from the background page). + DictionaryValue* associated_tab_value = NULL; + value->GetDictionary(keys::kAutomationTabJsonKey, &associated_tab_value); + DLOG(INFO) << "Request: " << request_id << ", for: " << function_name; FactoryMap::const_iterator iter = factories_.find(function_name); DCHECK(iter != factories_.end()); @@ -73,7 +79,7 @@ void ApiDispatcher::HandleApiRequest(BSTR message_text, BSTR* response) { scoped_ptr<Invocation> invocation(iter->second()); DCHECK(invocation.get()); - invocation->Execute(*args_list, request_id); + invocation->Execute(*args_list, request_id, associated_tab_value); } void ApiDispatcher::FireEvent(const char* event_name, const char* event_args) { @@ -149,6 +155,11 @@ HWND ApiDispatcher::GetTabHandleFromId(int tab_id) const { GetTabHandleFromId(tab_id); } +HWND ApiDispatcher::GetTabHandleFromToolBandId(int tool_band_id) const { + return Singleton<ExecutorsManager, ExecutorsManager::SingletonTraits>::get()-> + GetTabHandleFromToolBandId(tool_band_id); +} + HWND ApiDispatcher::GetWindowHandleFromId(int window_id) const { return reinterpret_cast<HWND>(window_id); } @@ -162,12 +173,6 @@ int ApiDispatcher::GetWindowIdFromHandle(HWND window_handle) const { return reinterpret_cast<int>(window_handle); } - -void ApiDispatcher::SetTabIdForHandle(long tab_id, HWND handle) { - Singleton<ExecutorsManager, ExecutorsManager::SingletonTraits>::get()-> - SetTabIdForHandle(tab_id, handle); -} - void ApiDispatcher::DeleteTabHandle(HWND handle) { Singleton<ExecutorsManager, ExecutorsManager::SingletonTraits>::get()-> DeleteTabHandle(handle); @@ -205,6 +210,10 @@ void ApiDispatcher::RegisterEphemeralEventHandler( } } +void ApiDispatcher::Invocation::Execute(const ListValue& args, int request_id) { + NOTREACHED(); +} + ApiDispatcher::InvocationResult::~InvocationResult() { // We must have posted the response before we died. DCHECK(request_id_ == kNoRequestId); diff --git a/ceee/ie/broker/api_dispatcher.h b/ceee/ie/broker/api_dispatcher.h index 9a651a4..d9fabde 100644 --- a/ceee/ie/broker/api_dispatcher.h +++ b/ceee/ie/broker/api_dispatcher.h @@ -121,10 +121,22 @@ class ApiDispatcher { virtual ~Invocation() {} // Called when a request to invoke the execution of an API is received. + // When an invocation does not need to access the associated tab, it should + // subclass this version of the Execute function. All APIs must subclass + // one of the 2 Execute member functions. // // @param args The list value object for the arguments passed. // @param request_id The identifier of the request being executed. - virtual void Execute(const ListValue& args, int request_id) = 0; + virtual void Execute(const ListValue& args, int request_id); + + // Called when a request to invoke the execution of an API is received. + // APIs that need the associated tab should implement this version of the + // Execute function directly; otherwise, this simply invokes the above + // version of Execute. + virtual void Execute(const ListValue& args, int request_id, + const DictionaryValue* associated_tab) { + Execute(args, request_id); + } protected: // Unit test seam. @@ -212,6 +224,13 @@ class ApiDispatcher { // found). virtual HWND GetTabHandleFromId(int tab_id) const; + // Return a tab handle associated with the given tool band tab id. + // + // @param tool_band_id The tab identifier for a tool band. + // @return The corresponding HWND (or INVALID_HANDLE_VALUE if tool_band_id + // isn't found). + virtual HWND GetTabHandleFromToolBandId(int tool_band_id) const; + // Return a window handle associated with the id. // // @param window_id The window identifier. @@ -231,9 +250,6 @@ class ApiDispatcher { // @return The corresponding window id (or 0 if window_handle isn't found). virtual int GetWindowIdFromHandle(HWND window_handle) const; - // Register the relation between a tab_id and a HWND. - virtual void SetTabIdForHandle(long tab_id, HWND tab_handle); - // Unregister the HWND and its corresponding tab_id. virtual void DeleteTabHandle(HWND handle); diff --git a/ceee/ie/broker/broker.cc b/ceee/ie/broker/broker.cc index 4d8ba22..36e8bc8 100644 --- a/ceee/ie/broker/broker.cc +++ b/ceee/ie/broker/broker.cc @@ -70,6 +70,17 @@ STDMETHODIMP CeeeBroker::SetTabIdForHandle(long tab_id, // TODO(mad@chromium.org): Add security check here. DCHECK(tab_id != kInvalidChromeSessionId && handle != reinterpret_cast<CeeeWindowHandle>(INVALID_HANDLE_VALUE)); - executors_manager_->SetTabIdForHandle(tab_id, reinterpret_cast<HWND>(handle)); + executors_manager_->SetTabIdForHandle( + static_cast<int>(tab_id), reinterpret_cast<HWND>(handle)); + return S_OK; +} + +STDMETHODIMP CeeeBroker::SetTabToolBandIdForHandle(long tool_band_id, + CeeeWindowHandle handle) { + // TODO(mad@chromium.org): Add security check here. + DCHECK(tool_band_id != kInvalidChromeSessionId && + handle != reinterpret_cast<CeeeWindowHandle>(INVALID_HANDLE_VALUE)); + executors_manager_->SetTabToolBandIdForHandle( + static_cast<int>(tool_band_id), reinterpret_cast<HWND>(handle)); return S_OK; } diff --git a/ceee/ie/broker/broker.h b/ceee/ie/broker/broker.h index 163757a..988301e 100644 --- a/ceee/ie/broker/broker.h +++ b/ceee/ie/broker/broker.h @@ -61,6 +61,8 @@ class ATL_NO_VTABLE CeeeBroker STDMETHOD(RegisterTabExecutor)(long thread_id, IUnknown* executor); STDMETHOD(UnregisterExecutor)(long thread_id); STDMETHOD(SetTabIdForHandle)(long tab_id, CeeeWindowHandle handle); + STDMETHOD(SetTabToolBandIdForHandle)(long tool_band_id, + CeeeWindowHandle handle); // @} // IExternalConnectionImpl overrides diff --git a/ceee/ie/broker/broker_lib.idl b/ceee/ie/broker/broker_lib.idl index eb0549c..00ae686 100644 --- a/ceee/ie/broker/broker_lib.idl +++ b/ceee/ie/broker/broker_lib.idl @@ -40,7 +40,7 @@ interface ICeeeBroker : IUnknown { [ object, - uuid(1821043a-a496-4d0e-9b50-1e0599237016), + uuid(FD0DFA82-AA64-43ef-8BFF-418941DEA26B), nonextensible, helpstring("ICeeeBrokerRegistrar Interface"), pointer_default(unique), @@ -76,16 +76,24 @@ interface ICeeeBrokerRegistrar : IUnknown { // TODO(hansl@google.com): Remove this and implement it in // RegisterTabExecutor. Make sure the Tab isn't registered before a TabId // is available. - // Link a tab_id with a related BHO handle. There is a strict one to one + // Link a tab_id with a related BHO handle. There is a strict one-to-one // relation between tab_ids and handles. // // @param tab_id The Chrome TabId related to the tab. // @param handle The HWND of the BHO for this TabId. HRESULT SetTabIdForHandle([in] long tab_id, [in] CeeeWindowHandle handle); + + // Link a tool band tab ID with a related BHO handle. There is a strict + // one-to-one relation between tab IDs and handles. + // + // @param tool_band_id The Chrome TabId related to the tab's tool band. + // @param handle The HWND of the BHO for this tab. + HRESULT SetTabToolBandIdForHandle([in] long tool_band_id, + [in] CeeeWindowHandle handle); }; [ - uuid(45B783D0-8040-49a6-A719-84E320AAB3C5), + uuid(7193CD84-54A1-46fb-A562-3EFB815F49BE), version(1.0), helpstring("CEEE Broker 1.0 Type Library") ] @@ -94,7 +102,7 @@ library CeeeBrokerLib { interface ICeeePostman; [ - uuid(6D88A70D-2218-4466-BBD6-87AB563811A2), + uuid(3E1AE932-28F6-4bfa-A831-D1E808CC4AF7), helpstring("CEEE Broker Class") ] coclass CeeeBroker { diff --git a/ceee/ie/broker/broker_unittest.cc b/ceee/ie/broker/broker_unittest.cc index aec7453..3ee3de9 100644 --- a/ceee/ie/broker/broker_unittest.cc +++ b/ceee/ie/broker/broker_unittest.cc @@ -27,6 +27,10 @@ class MockExecutorsManager : public ExecutorsManager { MOCK_METHOD2(RegisterTabExecutor, HRESULT(ThreadId thread_id, IUnknown* executor)); MOCK_METHOD1(RemoveExecutor, HRESULT(ThreadId thread_id)); + + MOCK_METHOD2(SetTabIdForHandle, void(int tab_id, HWND tab_handle)); + MOCK_METHOD2(SetTabToolBandIdForHandle, void(int tool_band_id, + HWND tab_handle)); }; class TestBroker: public CeeeBroker { @@ -72,6 +76,13 @@ TEST(CeeeBroker, All) { EXPECT_CALL(executors_manager, RemoveExecutor(0)). WillOnce(Return(S_OK)); EXPECT_EQ(S_OK, broker->UnregisterExecutor(0)); + + HWND test_handle = reinterpret_cast<HWND>(100); + EXPECT_CALL(executors_manager, SetTabIdForHandle(99, test_handle)); + EXPECT_EQ(S_OK, broker->SetTabIdForHandle(99, 100)); + + EXPECT_CALL(executors_manager, SetTabToolBandIdForHandle(99, test_handle)); + EXPECT_EQ(S_OK, broker->SetTabToolBandIdForHandle(99, 100)); } } // namespace diff --git a/ceee/ie/broker/executors_manager.cc b/ceee/ie/broker/executors_manager.cc index 6511d2d..b8effdf 100644 --- a/ceee/ie/broker/executors_manager.cc +++ b/ceee/ie/broker/executors_manager.cc @@ -295,41 +295,85 @@ HRESULT ExecutorsManager::Terminate() { return S_OK; } -void ExecutorsManager::SetTabIdForHandle(long tab_id, HWND handle) { +void ExecutorsManager::SetTabIdForHandle(int tab_id, HWND handle) { AutoLock lock(lock_); - DCHECK(tab_id_map_.end() == tab_id_map_.find(tab_id)); - DCHECK(handle_map_.end() == handle_map_.find(handle)); + if (tab_id_map_.end() != tab_id_map_.find(tab_id) || + handle_map_.end() != handle_map_.find(handle)) { + // Avoid double-setting of tab id -> handle mappings, which could otherwise + // lead to inconsistencies. In practice, this should never happen. + NOTREACHED(); + return; + } if (handle == reinterpret_cast<HWND>(INVALID_HANDLE_VALUE) || tab_id == kInvalidChromeSessionId) { NOTREACHED(); return; } + // A tool band tab ID should not be registered with this function. + DCHECK(tool_band_id_map_.end() == tool_band_id_map_.find(tab_id)); tab_id_map_[tab_id] = handle; handle_map_[handle] = tab_id; } -void ExecutorsManager::DeleteTabHandle(HWND handle) { +void ExecutorsManager::SetTabToolBandIdForHandle(int tool_band_id, + HWND handle) { AutoLock lock(lock_); - HandleMap::iterator handle_it = handle_map_.find(handle); - if(handle_map_.end() == handle_it) { - DCHECK(false); + if (tool_band_id_map_.end() != tool_band_id_map_.find(tool_band_id) || + tool_band_handle_map_.end() != tool_band_handle_map_.find(handle)) { + // Avoid double-setting of tool band id -> handle mappings, which could + // otherwise lead to inconsistencies. In practice, this should never + // happen. + NOTREACHED(); return; } - - TabIdMap::iterator tab_id_it = tab_id_map_.find(handle_it->second); - if(tab_id_map_.end() == tab_id_it) { - DCHECK(false); + if (handle == reinterpret_cast<HWND>(INVALID_HANDLE_VALUE) || + tool_band_id == kInvalidChromeSessionId) { + NOTREACHED(); return; } + // A BHO tab ID should not be registered with this function. + DCHECK(tab_id_map_.end() == tab_id_map_.find(tool_band_id)); + + tool_band_id_map_[tool_band_id] = handle; + tool_band_handle_map_[handle] = tool_band_id; +} +void ExecutorsManager::DeleteTabHandle(HWND handle) { + AutoLock lock(lock_); + HandleMap::iterator handle_it = handle_map_.find(handle); + DCHECK(handle_map_.end() != handle_it); + if (handle_map_.end() != handle_it) { + TabIdMap::iterator tab_id_it = tab_id_map_.find(handle_it->second); + DCHECK(tab_id_map_.end() != tab_id_it); + if (tab_id_map_.end() != tab_id_it) { #ifdef DEBUG - tab_id_map_[handle_it->second] = reinterpret_cast<HWND>(INVALID_HANDLE_VALUE); - handle_map_[handle] = kInvalidChromeSessionId; + tab_id_map_[handle_it->second] = + reinterpret_cast<HWND>(INVALID_HANDLE_VALUE); + handle_map_[handle] = kInvalidChromeSessionId; #else - tab_id_map_.erase(handle_it->second); - handle_map_.erase(handle); + tab_id_map_.erase(handle_it->second); + handle_map_.erase(handle); #endif // DEBUG + } + } + + handle_it = tool_band_handle_map_.find(handle); + if (tool_band_handle_map_.end() != handle_it) { + TabIdMap::iterator tool_band_id_it = + tool_band_id_map_.find(handle_it->second); + DCHECK(tool_band_id_map_.end() != tool_band_id_it); + if (tool_band_id_map_.end() != tool_band_id_it) { +#ifdef DEBUG + tool_band_id_map_[handle_it->second] = + reinterpret_cast<HWND>(INVALID_HANDLE_VALUE); + tool_band_handle_map_[handle] = kInvalidChromeSessionId; +#else + tool_band_id_map_.erase(handle_it->second); + tool_band_handle_map_.erase(handle); +#endif // DEBUG + } + } } HWND ExecutorsManager::GetTabHandleFromId(int tab_id) { @@ -355,6 +399,16 @@ int ExecutorsManager::GetTabIdFromHandle(HWND tab_handle) { return it->second; } +HWND ExecutorsManager::GetTabHandleFromToolBandId(int tool_band_id) { + AutoLock lock(lock_); + TabIdMap::const_iterator it = tool_band_id_map_.find(tool_band_id); + + if (it == tool_band_id_map_.end()) + return reinterpret_cast<HWND>(INVALID_HANDLE_VALUE); + + return it->second; +} + HRESULT ExecutorsManager::GetExecutorCreator( ICeeeExecutorCreator** executor_creator) { return ::CoCreateInstance(CLSID_CeeeExecutorCreator, NULL, diff --git a/ceee/ie/broker/executors_manager.h b/ceee/ie/broker/executors_manager.h index cba7b3f..05cb6ea 100644 --- a/ceee/ie/broker/executors_manager.h +++ b/ceee/ie/broker/executors_manager.h @@ -76,16 +76,26 @@ class ExecutorsManager { // found). virtual HWND GetTabHandleFromId(int tab_id); - // Return a tab id associated with the HWND. + // Return a tab ID associated with the HWND. // // @param tab_handle The tab HWND. - // @return The corresponding tab id (or 0 if tab_handle isn't found). + // @return The corresponding tab ID (or 0 if tab_handle isn't found). virtual int GetTabIdFromHandle(HWND tab_handle); - // Register the relation between a tab_id and a HWND. - virtual void SetTabIdForHandle(long tab_id, HWND tab_handle); + // Register the relation between a tab ID and an HWND. + virtual void SetTabIdForHandle(int tab_id, HWND tab_handle); - // Unregister the HWND and its corresponding tab_id. + // Return a tab handle associated with the given tool band tab ID. + // + // @param tool_band_id The tab identifier for a tool band. + // @return The corresponding HWND (or INVALID_HANDLE_VALUE if tool_band_id + // isn't found). + virtual HWND GetTabHandleFromToolBandId(int tool_band_id); + + // Register the relation between a tool band tab ID and an HWND. + virtual void SetTabToolBandIdForHandle(int tool_band_id, HWND tab_handle); + + // Unregister the HWND and its corresponding tab ID and tool band tab ID. virtual void DeleteTabHandle(HWND handle); // Traits for Singleton<ExecutorsManager> so that we can pass an argument @@ -179,7 +189,7 @@ class ExecutorsManager { // Thread protected by ExecutorsManager::lock_. Tid2Event pending_registrations_; - // The mapping between a tab_id and the HWND of the window holding the BHO. + // The mapping between a tab ID and the HWND of the window holding the BHO. // In DEBUG, this mapping will grow over time since we don't remove it on // DeleteTabHandle. This is useful for debugging as we know if a mapping has // been deleted and is invalidly used. @@ -187,6 +197,12 @@ class ExecutorsManager { TabIdMap tab_id_map_; HandleMap handle_map_; + // The mapping between a tool band's tab ID and the HWND of the window + // holding the BHO. + // Thread protected by ExecutorsManager::lock_. + TabIdMap tool_band_id_map_; + HandleMap tool_band_handle_map_; + // The handle to the thread running ThreadProc. CHandle thread_; @@ -200,7 +216,7 @@ class ExecutorsManager { // ExecutorsManager::pending_registrations_ & tab_id_map_/handle_map_). Lock lock_; - DISALLOW_EVIL_CONSTRUCTORS(ExecutorsManager); + DISALLOW_COPY_AND_ASSIGN(ExecutorsManager); }; #endif // CEEE_IE_BROKER_EXECUTORS_MANAGER_H_ diff --git a/ceee/ie/broker/executors_manager_unittest.cc b/ceee/ie/broker/executors_manager_unittest.cc index 47c6275..2fe3a5b 100644 --- a/ceee/ie/broker/executors_manager_unittest.cc +++ b/ceee/ie/broker/executors_manager_unittest.cc @@ -22,6 +22,11 @@ using testing::InvokeWithoutArgs; using testing::Return; using testing::SetArgumentPointee; +const int kTabWindowId = 99; +const int kTabWindowId2 = 88; +const HWND kTabWindow = reinterpret_cast<HWND>(kTabWindowId + 1); +const HWND kTabWindow2 = reinterpret_cast<HWND>(kTabWindowId2 + 1); + // We mock the IUnknown interface to make sure we properly AddRef and // Release the executors in the manager's map. // TODO(mad@chromium.org): replace this with the MockExecutorIUnknown @@ -661,4 +666,114 @@ TEST_F(ExecutorsManagerTests, ThreadProc) { EXPECT_EQ(1, executors_manager.CallThreadProc()); } +TEST_F(ExecutorsManagerTests, SetTabIdForHandle) { + testing::LogDisabler no_logs; + TestingExecutorsManager executors_manager; + + EXPECT_EQ(kInvalidChromeSessionId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(INVALID_HANDLE_VALUE, + executors_manager.GetTabHandleFromId(kTabWindowId)); + executors_manager.SetTabIdForHandle(kTabWindowId, kTabWindow); + EXPECT_EQ(kTabWindowId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(kTabWindow, + executors_manager.GetTabHandleFromId(kTabWindowId)); + // Tab IDs or handles can only be mapped once. + executors_manager.SetTabIdForHandle(kTabWindowId, kTabWindow2); + EXPECT_EQ(kInvalidChromeSessionId, + executors_manager.GetTabIdFromHandle(kTabWindow2)); + EXPECT_EQ(kTabWindowId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(kTabWindow, + executors_manager.GetTabHandleFromId(kTabWindowId)); + executors_manager.SetTabIdForHandle(kTabWindowId2, kTabWindow); + EXPECT_EQ(kTabWindowId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(kTabWindow, + executors_manager.GetTabHandleFromId(kTabWindowId)); + EXPECT_EQ(INVALID_HANDLE_VALUE, + executors_manager.GetTabHandleFromId(kTabWindowId2)); + executors_manager.SetTabIdForHandle(kTabWindowId2, kTabWindow2); + EXPECT_EQ(kTabWindowId2, + executors_manager.GetTabIdFromHandle(kTabWindow2)); + EXPECT_EQ(kTabWindow2, + executors_manager.GetTabHandleFromId(kTabWindowId2)); +} + +TEST_F(ExecutorsManagerTests, SetTabToolBandIdForHandle) { + testing::LogDisabler no_logs; + TestingExecutorsManager executors_manager; + + EXPECT_EQ(INVALID_HANDLE_VALUE, + executors_manager.GetTabHandleFromToolBandId(kTabWindowId)); + executors_manager.SetTabToolBandIdForHandle(kTabWindowId, kTabWindow); + EXPECT_EQ(kTabWindow, + executors_manager.GetTabHandleFromToolBandId(kTabWindowId)); + // Tool band tab IDs or handles can only be mapped once. + executors_manager.SetTabToolBandIdForHandle(kTabWindowId, kTabWindow2); + EXPECT_EQ(kTabWindow, + executors_manager.GetTabHandleFromToolBandId(kTabWindowId)); + executors_manager.SetTabToolBandIdForHandle(kTabWindowId2, kTabWindow); + EXPECT_EQ(INVALID_HANDLE_VALUE, + executors_manager.GetTabHandleFromToolBandId(kTabWindowId2)); + executors_manager.SetTabToolBandIdForHandle(kTabWindowId2, kTabWindow2); + EXPECT_EQ(kTabWindow2, + executors_manager.GetTabHandleFromToolBandId(kTabWindowId2)); +} + +TEST_F(ExecutorsManagerTests, DeleteTabHandle) { + testing::LogDisabler no_logs; + TestingExecutorsManager executors_manager; + + // Test deletion of a nonexistent window. + executors_manager.DeleteTabHandle(kTabWindow); + EXPECT_EQ(kInvalidChromeSessionId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(INVALID_HANDLE_VALUE, + executors_manager.GetTabHandleFromId(kTabWindowId)); + executors_manager.SetTabIdForHandle(kTabWindowId, kTabWindow); + EXPECT_EQ(kTabWindowId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(kTabWindow, + executors_manager.GetTabHandleFromId(kTabWindowId)); + executors_manager.DeleteTabHandle(kTabWindow); + EXPECT_EQ(kInvalidChromeSessionId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(INVALID_HANDLE_VALUE, + executors_manager.GetTabHandleFromId(kTabWindowId)); + // Test deletion of a nonexistent window when another one exists. + executors_manager.SetTabIdForHandle(kTabWindowId2, kTabWindow2); + executors_manager.DeleteTabHandle(kTabWindow); + EXPECT_EQ(kInvalidChromeSessionId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(INVALID_HANDLE_VALUE, + executors_manager.GetTabHandleFromId(kTabWindowId)); + EXPECT_EQ(kTabWindowId2, + executors_manager.GetTabIdFromHandle(kTabWindow2)); + EXPECT_EQ(kTabWindow2, + executors_manager.GetTabHandleFromId(kTabWindowId2)); +} + +TEST_F(ExecutorsManagerTests, DeleteTabHandleWithToolBandId) { + testing::LogDisabler no_logs; + TestingExecutorsManager executors_manager; + + executors_manager.SetTabIdForHandle(kTabWindowId, kTabWindow); + executors_manager.SetTabToolBandIdForHandle(kTabWindowId2, kTabWindow); + EXPECT_EQ(kTabWindowId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(kTabWindow, + executors_manager.GetTabHandleFromId(kTabWindowId)); + EXPECT_EQ(kTabWindow, + executors_manager.GetTabHandleFromToolBandId(kTabWindowId2)); + executors_manager.DeleteTabHandle(kTabWindow); + EXPECT_EQ(kInvalidChromeSessionId, + executors_manager.GetTabIdFromHandle(kTabWindow)); + EXPECT_EQ(INVALID_HANDLE_VALUE, + executors_manager.GetTabHandleFromId(kTabWindowId)); + EXPECT_EQ(INVALID_HANDLE_VALUE, + executors_manager.GetTabHandleFromToolBandId(kTabWindowId2)); +} + } // namespace diff --git a/ceee/ie/broker/tab_api_module.cc b/ceee/ie/broker/tab_api_module.cc index 7b8b6ff..9daa6f4 100644 --- a/ceee/ie/broker/tab_api_module.cc +++ b/ceee/ie/broker/tab_api_module.cc @@ -20,10 +20,12 @@ #include "base/json/json_reader.h" #include "base/json/json_writer.h" #include "base/logging.h" +#include "base/scoped_comptr_win.h" #include "base/string_number_conversions.h" #include "base/string_util.h" -#include "base/values.h" #include "base/utf_string_conversions.h" +#include "base/values.h" +#include "base/win/scoped_bstr.h" #include "base/win/windows_version.h" #include "ceee/ie/broker/api_module_constants.h" #include "ceee/ie/broker/api_module_util.h" @@ -120,7 +122,7 @@ bool CeeeUnmapTabEventHandler(const std::string& input_args, return false; } -} +} // namespace void RegisterInvocations(ApiDispatcher* dispatcher) { #define REGISTER_API_FUNCTION(func) do { dispatcher->RegisterInvocation(\ @@ -169,9 +171,9 @@ bool TabApiResult::CreateTabValue(int tab_id, long index) { return false; } - CComPtr<ICeeeTabExecutor> executor; + ScopedComPtr<ICeeeTabExecutor> executor; dispatcher->GetExecutor(tab_window, IID_ICeeeTabExecutor, - reinterpret_cast<void**>(&executor)); + reinterpret_cast<void**>(executor.Receive())); if (executor == NULL) { LOG(WARNING) << "Failed to get an executor to get tab info."; PostError(api_module_constants::kInternalErrorError); @@ -233,9 +235,9 @@ bool TabApiResult::CreateTabValue(int tab_id, long index) { // so we can save an IPC call. if (index == -1) { // We need another executor to get the index from the frame window thread. - CComPtr<ICeeeWindowExecutor> executor; + ScopedComPtr<ICeeeWindowExecutor> executor; dispatcher->GetExecutor(frame_window, IID_ICeeeWindowExecutor, - reinterpret_cast<void**>(&executor)); + reinterpret_cast<void**>(executor.Receive())); if (executor == NULL) { LOG(WARNING) << "Failed to get an executor to get tab index."; PostError(api_module_constants::kInternalErrorError); @@ -399,6 +401,41 @@ void GetTab::Execute(const ListValue& args, int request_id) { } } +void GetCurrentTab::Execute(const ListValue& args, int request_id, + const DictionaryValue* associated_tab) { + // TODO(cindylau@chromium.org): This implementation of chrome.tabs.getCurrent + // assumes that the associated tab will be that of a tool band, since that is + // the only way we currently support running extension code in a tab context. + // If the way we associate tool bands to IE tabs/windows changes, and/or if + // we add other tab-related extension widgets (e.g. infobars, or directly + // loading extension pages in host browser tabs), we will need to revisit + // this implementation. + scoped_ptr<TabApiResult> result(CreateApiResult(request_id)); + if (associated_tab == NULL) { + // The associated tab may validly be NULL, for instance if the API call + // originated from the background page. + result->PostResult(); + return; + } + int tool_band_id; + if (!associated_tab->GetInteger(ext::kIdKey, &tool_band_id)) { + result->PostError(api_module_constants::kInternalErrorError); + return; + } + + ApiDispatcher* dispatcher = GetDispatcher(); + DCHECK(dispatcher != NULL); + HWND tab_window = dispatcher->GetTabHandleFromToolBandId(tool_band_id); + int tab_id = dispatcher->GetTabIdFromHandle(tab_window); + if (!result->IsTabWindowClass(tab_window)) { + result->PostError(ExtensionErrorUtils::FormatErrorMessage( + ext::kTabNotFoundError, base::IntToString(tab_id))); + return; + } + if (result->CreateTabValue(tab_id, -1)) + result->PostResult(); +} + void GetSelectedTab::Execute(const ListValue& args, int request_id) { scoped_ptr<TabApiResult> result(CreateApiResult(request_id)); @@ -491,7 +528,7 @@ bool GetAllTabsInWindowResult::Execute(BSTR tab_handles) { size_t num_values = tabs_list->GetSize(); if (num_values % 2 != 0) { - // Values should come in pairs, one for the id and another one for the + // Values should come in pairs, one for the handle and another one for the // index. NOTREACHED() << "Invalid tabs list BSTR: " << tab_handles; PostError(api_module_constants::kInternalErrorError); @@ -529,9 +566,9 @@ void GetAllTabsInWindow::Execute(const ListValue& args, int request_id) { ApiDispatcher* dispatcher = GetDispatcher(); DCHECK(dispatcher != NULL); - CComPtr<ICeeeWindowExecutor> executor; + ScopedComPtr<ICeeeWindowExecutor> executor; dispatcher->GetExecutor(frame_window, IID_ICeeeWindowExecutor, - reinterpret_cast<void**>(&executor)); + reinterpret_cast<void**>(executor.Receive())); if (executor == NULL) { LOG(WARNING) << "Failed to get an executor to get list of tabs."; result->PostError("Internal Error while getting all tabs in window."); @@ -539,8 +576,8 @@ void GetAllTabsInWindow::Execute(const ListValue& args, int request_id) { } long num_tabs = 0; - CComBSTR tab_handles; - HRESULT hr = executor->GetTabs(&tab_handles); + base::win::ScopedBstr tab_handles; + HRESULT hr = executor->GetTabs(tab_handles.Receive()); if (FAILED(hr)) { DCHECK(tab_handles == NULL); LOG(ERROR) << "Failed to get list of tabs for window: " << std::hex << @@ -590,16 +627,16 @@ void UpdateTab::Execute(const ListValue& args, int request_id) { return; } - CComPtr<ICeeeTabExecutor> executor; + ScopedComPtr<ICeeeTabExecutor> executor; dispatcher->GetExecutor(tab_window, IID_ICeeeTabExecutor, - reinterpret_cast<void**>(&executor)); + reinterpret_cast<void**>(executor.Receive())); if (executor == NULL) { LOG(WARNING) << "Failed to get an executor to navigate tab."; result->PostError("Internal error trying to update tab."); return; } - HRESULT hr = executor->Navigate(CComBSTR(url.c_str()), 0, - CComBSTR(L"_top")); + HRESULT hr = executor->Navigate(base::win::ScopedBstr(url.c_str()), 0, + base::win::ScopedBstr(L"_top")); // Don't DCHECK here, see the comment at the bottom of // CeeeExecutor::Navigate(). if (FAILED(hr)) { @@ -620,10 +657,10 @@ void UpdateTab::Execute(const ListValue& args, int request_id) { // We only take action if the user wants to select the tab; this function // does not actually let you deselect a tab. if (selected) { - CComPtr<ICeeeWindowExecutor> frame_executor; - dispatcher->GetExecutor(window_utils::GetTopLevelParent(tab_window), - IID_ICeeeWindowExecutor, - reinterpret_cast<void**>(&frame_executor)); + ScopedComPtr<ICeeeWindowExecutor> frame_executor; + dispatcher->GetExecutor( + window_utils::GetTopLevelParent(tab_window), IID_ICeeeWindowExecutor, + reinterpret_cast<void**>(frame_executor.Receive())); if (frame_executor == NULL) { LOG(WARNING) << "Failed to get a frame executor to select tab."; result->PostError("Internal error trying to select tab."); @@ -663,10 +700,10 @@ void RemoveTab::Execute(const ListValue& args, int request_id) { return; } - CComPtr<ICeeeWindowExecutor> frame_executor; + ScopedComPtr<ICeeeWindowExecutor> frame_executor; dispatcher->GetExecutor(window_utils::GetTopLevelParent(tab_window), IID_ICeeeWindowExecutor, - reinterpret_cast<void**>(&frame_executor)); + reinterpret_cast<void**>(frame_executor.Receive())); if (frame_executor == NULL) { LOG(WARNING) << "Failed to get a frame executor to select tab."; result->PostError("Internal error trying to select tab."); @@ -797,9 +834,9 @@ void CreateTab::Execute(const ListValue& args, int request_id) { // So bb2284073 & bb2492252 might still occur there. std::wstring url_wstring = UTF8ToWide(url_string); if (base::win::GetVersion() < base::win::VERSION_VISTA) { - CComPtr<IWebBrowser2> browser; + ScopedComPtr<IWebBrowser2> browser; HRESULT hr = ie_util::GetWebBrowserForTopLevelIeHwnd( - frame_window, NULL, &browser); + frame_window, NULL, browser.Receive()); DCHECK(SUCCEEDED(hr)) << "Can't get the browser for window: " << frame_window; if (FAILED(hr)) { @@ -808,7 +845,7 @@ void CreateTab::Execute(const ListValue& args, int request_id) { } long flags = selected ? navOpenInNewTab : navOpenInBackgroundTab; - hr = browser->Navigate(CComBSTR(url_wstring.c_str()), + hr = browser->Navigate(base::win::ScopedBstr(url_wstring.c_str()), &CComVariant(flags), &CComVariant(L"_blank"), &CComVariant(), // Post data @@ -827,9 +864,9 @@ void CreateTab::Execute(const ListValue& args, int request_id) { DCHECK(success && existing_tab != NULL) << "Can't find an existing tab for" << frame_window; - CComPtr<ICeeeTabExecutor> executor; + ScopedComPtr<ICeeeTabExecutor> executor; dispatcher->GetExecutor(existing_tab, IID_ICeeeTabExecutor, - reinterpret_cast<void**>(&executor)); + reinterpret_cast<void**>(executor.Receive())); if (executor == NULL) { LOG(WARNING) << "Failed to get an executor to create a tab."; result->PostError("Internal error while trying to create tab."); @@ -837,8 +874,8 @@ void CreateTab::Execute(const ListValue& args, int request_id) { } long flags = selected ? navOpenInNewTab : navOpenInBackgroundTab; - HRESULT hr = executor->Navigate(CComBSTR(url_wstring.c_str()), - flags, CComBSTR(L"_blank")); + HRESULT hr = executor->Navigate(base::win::ScopedBstr(url_wstring.c_str()), + flags, base::win::ScopedBstr(L"_blank")); // We can DCHECK here because navigating to a new tab shouldn't fail as // described in the comment at the bottom of CeeeExecutor::Navigate(). DCHECK(SUCCEEDED(hr)) << "Failed to create tab. " << com::LogHr(hr); @@ -915,9 +952,9 @@ HRESULT CreateTab::ContinueExecution(const std::string& input_args, DCHECK(success) << "index_value->GetAsInteger()"; HWND frame_window = window_utils::GetTopLevelParent(tab_window); - CComPtr<ICeeeWindowExecutor> frame_executor; + ScopedComPtr<ICeeeWindowExecutor> frame_executor; dispatcher->GetExecutor(frame_window, __uuidof(ICeeeWindowExecutor), - reinterpret_cast<void**>(&frame_executor)); + reinterpret_cast<void**>(frame_executor.Receive())); if (frame_executor == NULL) { LOG(WARNING) << "Failed to get an executor for the frame."; result->PostError("Internal error while trying to move created tab."); @@ -1032,9 +1069,9 @@ void MoveTab::Execute(const ListValue& args, int request_id) { HWND frame_window = window_utils::GetTopLevelParent(tab_window); - CComPtr<ICeeeWindowExecutor> frame_executor; + ScopedComPtr<ICeeeWindowExecutor> frame_executor; dispatcher->GetExecutor(frame_window, IID_ICeeeWindowExecutor, - reinterpret_cast<void**>(&frame_executor)); + reinterpret_cast<void**>(frame_executor.Receive())); if (frame_executor == NULL) { LOG(WARNING) << "Failed to get an executor for the frame."; result->PostError("Internal Error while trying to move tab."); @@ -1104,9 +1141,9 @@ ApiDispatcher::InvocationResult* TabsInsertCode::ExecuteImpl( return NULL; } - CComPtr<ICeeeTabExecutor> tab_executor; + ScopedComPtr<ICeeeTabExecutor> tab_executor; dispatcher->GetExecutor(tab_window, IID_ICeeeTabExecutor, - reinterpret_cast<void**>(&tab_executor)); + reinterpret_cast<void**>(tab_executor.Receive())); if (tab_executor == NULL) { LOG(WARNING) << "Failed to get an executor for the frame."; result->PostError("Internal Error while trying to insert code in tab."); diff --git a/ceee/ie/broker/tab_api_module.h b/ceee/ie/broker/tab_api_module.h index 50c0f69..b40f7f8 100644 --- a/ceee/ie/broker/tab_api_module.h +++ b/ceee/ie/broker/tab_api_module.h @@ -59,6 +59,12 @@ class GetTab : public TabApiResultCreator { virtual void Execute(const ListValue& args, int request_id); }; +class GetCurrentTab : public TabApiResultCreator { + public: + virtual void Execute(const ListValue& args, int request_id, + const DictionaryValue* associated_tab); +}; + class GetSelectedTab : public TabApiResultCreator { public: virtual void Execute(const ListValue& args, int request_id); diff --git a/ceee/ie/broker/tab_api_module_unittest.cc b/ceee/ie/broker/tab_api_module_unittest.cc index 8f1f4df..a27ba82 100644 --- a/ceee/ie/broker/tab_api_module_unittest.cc +++ b/ceee/ie/broker/tab_api_module_unittest.cc @@ -10,6 +10,7 @@ #include "base/json/json_writer.h" #include "base/json/json_reader.h" +#include "base/scoped_comptr_win.h" #include "base/win/windows_version.h" #include "ceee/common/initializing_coclass.h" #include "ceee/ie/broker/api_dispatcher.h" @@ -35,6 +36,7 @@ namespace { using tab_api::CreateTab; using tab_api::GetAllTabsInWindow; using tab_api::GetAllTabsInWindowResult; +using tab_api::GetCurrentTab; using tab_api::GetSelectedTab; using tab_api::GetTab; using tab_api::MoveTab; @@ -115,9 +117,9 @@ class TabApiTests: public testing::Test { public: virtual void SetUp() { EXPECT_HRESULT_SUCCEEDED(testing::MockTabExecutor::CreateInitialized( - &mock_tab_executor_, &mock_tab_executor_keeper_)); + &mock_tab_executor_, mock_tab_executor_keeper_.Receive())); EXPECT_HRESULT_SUCCEEDED(testing::MockWindowExecutor::CreateInitialized( - &mock_window_executor_, &mock_window_executor_keeper_)); + &mock_window_executor_, mock_window_executor_keeper_.Receive())); } virtual void TearDown() { @@ -134,32 +136,32 @@ class TabApiTests: public testing::Test { // void** argument instead of an interface. EXPECT_CALL(*api_dispatcher, GetExecutor(window, _, NotNull())). WillRepeatedly(DoAll( - SetArgumentPointee<2>(mock_tab_executor_keeper_.p), - AddRef(mock_tab_executor_keeper_.p))); + SetArgumentPointee<2>(mock_tab_executor_keeper_.get()), + AddRef(mock_tab_executor_keeper_.get()))); } void MockGetTabExecutorOnce(MockApiDispatcher* api_dispatcher, HWND window) { // We can't use CopyInterfaceToArgument here because GetExecutor takes a // void** argument instead of an interface. EXPECT_CALL(*api_dispatcher, GetExecutor(window, _, NotNull())). - WillOnce(DoAll(SetArgumentPointee<2>(mock_tab_executor_keeper_.p), - AddRef(mock_tab_executor_keeper_.p))); + WillOnce(DoAll(SetArgumentPointee<2>(mock_tab_executor_keeper_.get()), + AddRef(mock_tab_executor_keeper_.get()))); } void MockGetWindowExecutorOnce(MockApiDispatcher* api_dispatcher, HWND window) { EXPECT_CALL(*api_dispatcher, GetExecutor(window, _, NotNull())). WillOnce(DoAll( - SetArgumentPointee<2>(mock_window_executor_keeper_.p), - AddRef(mock_window_executor_keeper_.p))); + SetArgumentPointee<2>(mock_window_executor_keeper_.get()), + AddRef(mock_window_executor_keeper_.get()))); } void AlwaysMockGetWindowExecutor(MockApiDispatcher* api_dispatcher, HWND window) { EXPECT_CALL(*api_dispatcher, GetExecutor(window, _, NotNull())). WillRepeatedly(DoAll( - SetArgumentPointee<2>(mock_window_executor_keeper_.p), - AddRef(mock_window_executor_keeper_.p))); + SetArgumentPointee<2>(mock_window_executor_keeper_.get()), + AddRef(mock_window_executor_keeper_.get()))); } StrictMock<testing::MockUser32> user32_; @@ -177,8 +179,8 @@ class TabApiTests: public testing::Test { // one and only singleton to use all the time. CComObjectStackEx< StrictMock< MockChromePostman > > postman_; // To control the life span of the tab executor. - CComPtr<ICeeeTabExecutor> mock_tab_executor_keeper_; - CComPtr<ICeeeWindowExecutor> mock_window_executor_keeper_; + ScopedComPtr<ICeeeTabExecutor> mock_tab_executor_keeper_; + ScopedComPtr<ICeeeWindowExecutor> mock_window_executor_keeper_; }; TEST_F(TabApiTests, CreateTabValueErrorHandling) { @@ -537,6 +539,52 @@ TEST_F(TabApiTests, GetTab) { invocation.Execute(ListValue(), kRequestId); } +TEST_F(TabApiTests, GetCurrentTab) { + testing::LogDisabler no_dchecks; + + // Mocking IsTabWindowClass. + StrictMock<testing::MockWindowUtils> window_utils; + EXPECT_CALL(window_utils, IsWindowClass(kGoodTabWindow, _)). + WillRepeatedly(Return(true)); + EXPECT_CALL(window_utils, IsWindowClass(kBadWindow, _)). + WillRepeatedly(Return(false)); + + StrictMock<MockApiInvocation<TabApiResult, MockTabApiResult, GetCurrentTab> > + invocation; + + // Start with success--no context. + invocation.AllocateNewResult(kRequestId); + EXPECT_CALL(*invocation.invocation_result_, PostResult()).Times(1); + invocation.Execute(ListValue(), kRequestId, NULL); + + // Now successfully get a current tab. + invocation.AllocateNewResult(kRequestId); + EXPECT_CALL(invocation.mock_api_dispatcher_, GetTabHandleFromToolBandId(3)). + WillOnce(Return(kGoodTabWindow)); + EXPECT_CALL(invocation.mock_api_dispatcher_, + GetTabIdFromHandle(kGoodTabWindow)).WillOnce(Return(kGoodTabWindowId)); + EXPECT_CALL(*invocation.invocation_result_, + CreateTabValue(kGoodTabWindowId, -1)).WillOnce(Return(true)); + EXPECT_CALL(*invocation.invocation_result_, PostResult()).Times(1); + DictionaryValue associated_tab; + associated_tab.SetInteger(ext::kIdKey, 3); + invocation.Execute(ListValue(), kRequestId, &associated_tab); + + // No more successful calls. + invocation.AllocateNewResult(kRequestId); + EXPECT_CALL(invocation.mock_api_dispatcher_, GetTabHandleFromToolBandId(3)). + WillOnce(Return(kBadWindow)); + EXPECT_CALL(invocation.mock_api_dispatcher_, + GetTabIdFromHandle(kBadWindow)).WillOnce(Return(kBadWindowId)); + EXPECT_CALL(*invocation.invocation_result_, PostError(_)).Times(1); + invocation.Execute(ListValue(), kRequestId, &associated_tab); + + invocation.AllocateNewResult(kRequestId); + EXPECT_CALL(*invocation.invocation_result_, PostError(_)).Times(1); + associated_tab.Remove(ext::kIdKey, NULL); + invocation.Execute(ListValue(), kRequestId, &associated_tab); +} + TEST_F(TabApiTests, GetSelectedTab) { testing::LogDisabler no_dchecks; StrictMock<MockApiInvocation<TabApiResult, MockTabApiResult, GetSelectedTab> > @@ -695,14 +743,14 @@ TEST_F(TabApiTests, GetAllTabsInWindow) { // Failing Executor. // The executor classes are already strict from their base class impl. testing::MockWindowExecutor* mock_window_executor; - CComPtr<ICeeeWindowExecutor> mock_window_executor_keeper_; + ScopedComPtr<ICeeeWindowExecutor> mock_window_executor_keeper_; EXPECT_HRESULT_SUCCEEDED(testing::MockWindowExecutor::CreateInitialized( - &mock_window_executor, &mock_window_executor_keeper_)); + &mock_window_executor, mock_window_executor_keeper_.Receive())); EXPECT_CALL(invocation.mock_api_dispatcher_, GetExecutor(kGoodFrameWindow, _, NotNull())). WillRepeatedly(DoAll(SetArgumentPointee<2>( - mock_window_executor_keeper_.p), - AddRef(mock_window_executor_keeper_.p))); + mock_window_executor_keeper_.get()), + AddRef(mock_window_executor_keeper_.get()))); EXPECT_CALL(*mock_window_executor, GetTabs(NotNull())). WillOnce(Return(E_FAIL)); EXPECT_CALL(*invocation.invocation_result_, PostError(_)).Times(1); @@ -1037,11 +1085,11 @@ TEST_F(TabApiTests, CreateTabExecute) { CComObject<StrictMock<testing::MockIWebBrowser2>>::CreateInstance( &browser); DCHECK(browser != NULL); - CComPtr<IWebBrowser2> browser_keeper = browser; + ScopedComPtr<IWebBrowser2> browser_keeper(browser); if (pre_vista) { EXPECT_CALL(mock_ie_util, GetWebBrowserForTopLevelIeHwnd( kGoodFrameWindow, _, NotNull())).WillRepeatedly(DoAll( - CopyInterfaceToArgument<2>(browser_keeper.p), Return(S_OK))); + CopyInterfaceToArgument<2>(browser_keeper.get()), Return(S_OK))); EXPECT_CALL(*browser, Navigate(_, _, _, _, _)).WillOnce(Return(E_FAIL)); } else { AlwaysMockGetTabExecutor(&invocation.mock_api_dispatcher_, kGoodTabWindow); diff --git a/ceee/ie/broker/window_api_module.cc b/ceee/ie/broker/window_api_module.cc index 2115163..2a50676 100644 --- a/ceee/ie/broker/window_api_module.cc +++ b/ceee/ie/broker/window_api_module.cc @@ -15,9 +15,11 @@ #include "base/json/json_reader.h" #include "base/logging.h" +#include "base/scoped_comptr_win.h" #include "base/string_number_conversions.h" #include "base/string_util.h" #include "base/utf_string_conversions.h" +#include "base/win/scoped_bstr.h" #include "ceee/common/com_utils.h" #include "ceee/common/process_utils_win.h" #include "ceee/common/windows_constants.h" @@ -119,9 +121,9 @@ bool WindowApiResult::UpdateWindowRect(HWND window, common_api::WindowInfo window_info; if (left != -1 || top != -1 || width != -1 || height != -1) { - CComPtr<ICeeeWindowExecutor> executor; + ScopedComPtr<ICeeeWindowExecutor> executor; dispatcher->GetExecutor(window, IID_ICeeeWindowExecutor, - reinterpret_cast<void**>(&executor)); + reinterpret_cast<void**>(executor.Receive())); if (executor == NULL) { LOG(WARNING) << "Failed to get an executor to update window."; PostError(api_module_constants::kInternalErrorError); @@ -159,12 +161,44 @@ void GetWindow::Execute(const ListValue& args, int request_id) { result->PostResult(); } -void GetCurrentWindow::Execute(const ListValue& args, int request_id) { - // TODO(mad@chromium.org): We currently don't have access to the - // actual 'current' window from the point of view of the extension - // API caller. Use the top window for now. bb2255140 +void GetCurrentWindow::Execute(const ListValue& args, int request_id, + const DictionaryValue* associated_tab) { scoped_ptr<WindowApiResult> result(CreateApiResult(request_id)); - if (result->CreateWindowValue(result->TopIeWindow(), false)) + if (associated_tab == NULL) { + // The associated tab may validly be NULL, for instance if the API call + // originated from the background page. If this happens, simply return + // the top-level window instead. + if (result->CreateWindowValue(result->TopIeWindow(), false)) + result->PostResult(); + return; + } + int tool_band_id; + if (!associated_tab->GetInteger(ext::kIdKey, &tool_band_id)) { + result->PostError(api_module_constants::kInternalErrorError); + return; + } + + ApiDispatcher* dispatcher = GetDispatcher(); + DCHECK(dispatcher != NULL); + HWND tab_window = dispatcher->GetTabHandleFromToolBandId(tool_band_id); + // The window ID is just the window handle of the frame window, which is the + // top-level ancestor of this window. + HWND frame_window = window_utils::GetTopLevelParent(tab_window); + if (frame_window == tab_window || + !result->IsTabWindowClass(tab_window) || + !result->IsIeFrameClass(frame_window)) { + // If we couldn't get a valid parent frame window, then it must be because + // the frame window (and the tab then) has been closed by now or it lives + // under the hidden IE window. + DCHECK(!::IsWindow(tab_window) || window_utils::IsWindowClass(frame_window, + windows::kHiddenIeFrameWindowClass)); + int tab_id = dispatcher->GetTabIdFromHandle(tab_window); + result->PostError(ExtensionErrorUtils::FormatErrorMessage( + ext::kTabNotFoundError, base::IntToString(tab_id))); + return; + } + + if (result->CreateWindowValue(frame_window, false)) result->PostResult(); } @@ -399,7 +433,7 @@ void CreateWindowFunc::Execute(const ListValue& args, int request_id) { } // Now we can Navigate to the requested url. - hr = web_browser->Navigate(CComBSTR(url.c_str()), + hr = web_browser->Navigate(base::win::ScopedBstr(url.c_str()), &CComVariant(), // unused flags &CComVariant(L"_top"), // Target frame &CComVariant(), // Unused POST DATA @@ -466,9 +500,9 @@ void RemoveWindow::Execute(const ListValue& args, int request_id) { return; } - CComPtr<ICeeeWindowExecutor> executor; + ScopedComPtr<ICeeeWindowExecutor> executor; dispatcher->GetExecutor(window, IID_ICeeeWindowExecutor, - reinterpret_cast<void**>(&executor)); + reinterpret_cast<void**>(executor.Receive())); if (executor == NULL) { LOG(WARNING) << "Failed to get an executor to remove window."; result->PostError(api_module_constants::kInternalErrorError); diff --git a/ceee/ie/broker/window_api_module.h b/ceee/ie/broker/window_api_module.h index 6aed95f..7d98e8d 100644 --- a/ceee/ie/broker/window_api_module.h +++ b/ceee/ie/broker/window_api_module.h @@ -46,7 +46,8 @@ class GetWindow : public WindowApiResultCreator { class GetCurrentWindow : public WindowApiResultCreator { public: - virtual void Execute(const ListValue& args, int request_id); + virtual void Execute(const ListValue& args, int request_id, + const DictionaryValue* associated_tab); }; class GetLastFocusedWindow : public WindowApiResultCreator { diff --git a/ceee/ie/broker/window_api_module_unittest.cc b/ceee/ie/broker/window_api_module_unittest.cc index c6972ef..0293f32 100644 --- a/ceee/ie/broker/window_api_module_unittest.cc +++ b/ceee/ie/broker/window_api_module_unittest.cc @@ -11,6 +11,7 @@ #include <iepmapi.h> #include <set> +#include "base/scoped_comptr_win.h" #include "base/scoped_ptr.h" #include "ceee/common/process_utils_win.h" #include "ceee/ie/broker/chrome_postman.h" @@ -20,7 +21,6 @@ #include "ceee/testing/utils/instance_count_mixin.h" #include "ceee/testing/utils/mock_com.h" #include "ceee/testing/utils/mock_window_utils.h" -#include "ceee/testing/utils/mock_win32.h" #include "ceee/testing/utils/test_utils.h" #include "chrome/browser/extensions/extension_event_names.h" #include "chrome/browser/extensions/extension_tabs_module_constants.h" @@ -208,9 +208,15 @@ int MockIterativeWindowApiResult::error_counter_ = 0; // Mock static functions defined in WindowApiResult. MOCK_STATIC_CLASS_BEGIN(MockWindowApiResultStatics) MOCK_STATIC_INIT_BEGIN(MockWindowApiResultStatics) + MOCK_STATIC_INIT2(WindowApiResult::IsTabWindowClass, + IsTabWindowClass); + MOCK_STATIC_INIT2(WindowApiResult::IsIeFrameClass, + IsIeFrameClass); MOCK_STATIC_INIT2(WindowApiResult::TopIeWindow, TopIeWindow); MOCK_STATIC_INIT_END() + MOCK_STATIC1(bool, , IsTabWindowClass, HWND); + MOCK_STATIC1(bool, , IsIeFrameClass, HWND); MOCK_STATIC0(HWND, , TopIeWindow); MOCK_STATIC_CLASS_END(MockWindowApiResultStatics) @@ -218,9 +224,9 @@ class WindowApiTests: public testing::Test { public: virtual void SetUp() { EXPECT_HRESULT_SUCCEEDED(testing::MockWindowExecutor::CreateInitialized( - &mock_window_executor_, &mock_window_executor_keeper_)); + &mock_window_executor_, mock_window_executor_keeper_.Receive())); EXPECT_HRESULT_SUCCEEDED(testing::MockTabExecutor::CreateInitialized( - &mock_tab_executor_, &mock_tab_executor_keeper_)); + &mock_tab_executor_, mock_tab_executor_keeper_.Receive())); } virtual void TearDown() { @@ -233,15 +239,15 @@ class WindowApiTests: public testing::Test { void MockGetWindowExecutor(MockApiDispatcher* executor_owner) { EXPECT_CALL(*executor_owner, GetExecutor(_, _, NotNull())). WillRepeatedly(DoAll( - SetArgumentPointee<2>(mock_window_executor_keeper_.p), - AddRef(mock_window_executor_keeper_.p))); + SetArgumentPointee<2>(mock_window_executor_keeper_.get()), + AddRef(mock_window_executor_keeper_.get()))); } void MockGetTabExecutor(MockApiDispatcher* executor_owner) { EXPECT_CALL(*executor_owner, GetExecutor(_, _, NotNull())). WillRepeatedly(DoAll( - SetArgumentPointee<2>(mock_tab_executor_keeper_.p), - AddRef(mock_tab_executor_keeper_.p))); + SetArgumentPointee<2>(mock_tab_executor_keeper_.get()), + AddRef(mock_tab_executor_keeper_.get()))); } void WindowAlwaysHasThread() { @@ -261,8 +267,8 @@ class WindowApiTests: public testing::Test { testing::MockWindowExecutor* mock_window_executor_; testing::MockTabExecutor* mock_tab_executor_; // To control the life span of the executors. - CComPtr<ICeeeWindowExecutor> mock_window_executor_keeper_; - CComPtr<ICeeeTabExecutor> mock_tab_executor_keeper_; + ScopedComPtr<ICeeeWindowExecutor> mock_window_executor_keeper_; + ScopedComPtr<ICeeeTabExecutor> mock_tab_executor_keeper_; private: class MockChromePostman : public ChromePostman { @@ -554,20 +560,99 @@ TEST_F(WindowApiTests, GetWindowStraightline) { invocation.Execute(args_list, kRequestId); } -TEST_F(WindowApiTests, GetCurrentWindowStraightline) { +TEST_F(WindowApiTests, GetCurrentWindowErrorHandling) { testing::LogDisabler no_dchecks; + StrictMock<MockWindowInvocation<window_api::GetCurrentWindow>> invocation; + StrictMock<testing::MockWindowUtils> window_utils; + MockWindowApiResultStatics result_statics; + DictionaryValue associated_tab; - EXPECT_CALL(user32_, IsWindow(_)).WillRepeatedly(Return(TRUE)); + EXPECT_CALL(user32_, IsWindow(_)).WillRepeatedly(Return(FALSE)); + + // Bad args failure. + invocation.AllocateNewResult(kRequestId); + EXPECT_CALL(*invocation.invocation_result_, PostError(_)).Times(1); + invocation.Execute(ListValue(), kRequestId, &associated_tab); + + associated_tab.SetInteger(keys::kIdKey, 3); + + // Bad window failures. + invocation.AllocateNewResult(kRequestId); + EXPECT_CALL(invocation.mock_api_dispatcher_, GetTabHandleFromToolBandId(3)). + WillOnce(Return(kRandomWindowHwnd)); + EXPECT_CALL(window_utils, GetTopLevelParent(kRandomWindowHwnd)). + WillOnce(Return(kRandomWindowHwnd)); + EXPECT_CALL(invocation.mock_api_dispatcher_, + GetTabIdFromHandle(kRandomWindowHwnd)). + WillOnce(Return(kRandomWindowId)); + EXPECT_CALL(*invocation.invocation_result_, PostError(_)).Times(1); + invocation.Execute(ListValue(), kRequestId, &associated_tab); + + invocation.AllocateNewResult(kRequestId); + EXPECT_CALL(invocation.mock_api_dispatcher_, GetTabHandleFromToolBandId(3)). + WillOnce(Return(kRandomWindowHwnd)); + EXPECT_CALL(window_utils, GetTopLevelParent(kRandomWindowHwnd)). + WillOnce(Return(kWindowHwnd)); + EXPECT_CALL(result_statics, IsTabWindowClass(kRandomWindowHwnd)). + WillOnce(Return(FALSE)); + EXPECT_CALL(invocation.mock_api_dispatcher_, + GetTabIdFromHandle(kRandomWindowHwnd)). + WillOnce(Return(kRandomWindowId)); + EXPECT_CALL(*invocation.invocation_result_, PostError(_)).Times(1); + invocation.Execute(ListValue(), kRequestId, &associated_tab); + + invocation.AllocateNewResult(kRequestId); + EXPECT_CALL(invocation.mock_api_dispatcher_, GetTabHandleFromToolBandId(3)). + WillOnce(Return(kRandomWindowHwnd)); + EXPECT_CALL(window_utils, GetTopLevelParent(kRandomWindowHwnd)). + WillOnce(Return(kWindowHwnd)); + EXPECT_CALL(result_statics, IsTabWindowClass(kRandomWindowHwnd)). + WillOnce(Return(TRUE)); + EXPECT_CALL(result_statics, IsIeFrameClass(kWindowHwnd)). + WillOnce(Return(FALSE)); + EXPECT_CALL(invocation.mock_api_dispatcher_, + GetTabIdFromHandle(kRandomWindowHwnd)). + WillOnce(Return(kRandomWindowId)); + EXPECT_CALL(*invocation.invocation_result_, PostError(_)).Times(1); + invocation.Execute(ListValue(), kRequestId, &associated_tab); +} + +TEST_F(WindowApiTests, GetCurrentWindowStraightline) { + testing::LogDisabler no_dchecks; StrictMock<MockWindowInvocation<window_api::GetCurrentWindow>> invocation; + MockWindowApiResultStatics result_statics; invocation.AllocateNewResult(kRequestId); + + EXPECT_CALL(invocation.mock_api_dispatcher_, GetTabHandleFromToolBandId(3)). + WillOnce(Return(kRandomWindowHwnd)); + StrictMock<testing::MockWindowUtils> window_utils; + EXPECT_CALL(window_utils, GetTopLevelParent(kRandomWindowHwnd)). + WillOnce(Return(kWindowHwnd)); + EXPECT_CALL(result_statics, IsTabWindowClass(kRandomWindowHwnd)). + WillOnce(Return(TRUE)); + EXPECT_CALL(result_statics, IsIeFrameClass(kWindowHwnd)). + WillOnce(Return(TRUE)); + EXPECT_CALL(*invocation.invocation_result_, + CreateWindowValue(kWindowHwnd, false)).WillOnce(Return(true)); + EXPECT_CALL(*invocation.invocation_result_, PostResult()).Times(1); + DictionaryValue associated_tab; + associated_tab.SetInteger(keys::kIdKey, 3); + invocation.Execute(ListValue(), kRequestId, &associated_tab); +} + +TEST_F(WindowApiTests, GetCurrentWindowNoContext) { + testing::LogDisabler no_dchecks; + StrictMock<MockWindowInvocation<window_api::GetCurrentWindow>> invocation; MockWindowApiResultStatics result_statics; + + invocation.AllocateNewResult(kRequestId); EXPECT_CALL(result_statics, TopIeWindow()).WillOnce(Return( kRandomWindowHwnd)); EXPECT_CALL(*invocation.invocation_result_, CreateWindowValue( kRandomWindowHwnd, _)).WillOnce(Return(true)); EXPECT_CALL(*invocation.invocation_result_, PostResult()).Times(1); - invocation.Execute(ListValue(), kRequestId); + invocation.Execute(ListValue(), kRequestId, NULL); } TEST_F(WindowApiTests, GetLastFocusedWindowStraightline) { @@ -643,10 +728,10 @@ TEST_F(WindowApiTests, CreateWindowErrorHandling) { CComObject<StrictMock<testing::MockIWebBrowser2>>* browser; CComObject<StrictMock<testing::MockIWebBrowser2>>::CreateInstance(&browser); DCHECK(browser != NULL); - CComPtr<IWebBrowser2> browser_keeper = browser; + ScopedComPtr<IWebBrowser2> browser_keeper(browser); EXPECT_CALL(mock_ie_create, CoCreateInstance(_, _, _, _, _)). - WillRepeatedly(DoAll(SetArgumentPointee<4>(browser_keeper.p), - AddRef(browser_keeper.p), Return(S_OK))); + WillRepeatedly(DoAll(SetArgumentPointee<4>(browser_keeper.get()), + AddRef(browser_keeper.get()), Return(S_OK))); EXPECT_CALL(*browser, get_HWND(NotNull())).WillRepeatedly(DoAll( SetArgumentPointee<0>(0), Return(S_OK))); @@ -708,14 +793,14 @@ TEST_F(WindowApiTests, CreateWindowStraightline) { CComObject<StrictMock<testing::MockIWebBrowser2>>* browser; CComObject<StrictMock<testing::MockIWebBrowser2>>::CreateInstance(&browser); DCHECK(browser != NULL); - CComPtr<IWebBrowser2> browser_keeper = browser; + ScopedComPtr<IWebBrowser2> browser_keeper(browser); MockIeWindowCreation mock_ie_create; // TODO(mad@chromium.org): Test behavior with protected on too. EXPECT_CALL(mock_ie_create, IEIsProtectedModeURL(_)). WillRepeatedly(Return(S_FALSE)); EXPECT_CALL(mock_ie_create, CoCreateInstance(_, _, _, _, _)). - WillRepeatedly(DoAll(SetArgumentPointee<4>(browser_keeper.p), - AddRef(browser_keeper.p), Return(S_OK))); + WillRepeatedly(DoAll(SetArgumentPointee<4>(browser_keeper.get()), + AddRef(browser_keeper.get()), Return(S_OK))); EXPECT_CALL(*browser, get_HWND(NotNull())).WillRepeatedly(DoAll( SetArgumentPointee<0>(0), Return(S_OK))); EXPECT_CALL(*browser, Navigate(_, _, _, _, _)).WillRepeatedly(Return(S_OK)); diff --git a/ceee/ie/common/api_registration.h b/ceee/ie/common/api_registration.h index 26dad37..77c227e 100644 --- a/ceee/ie/common/api_registration.h +++ b/ceee/ie/common/api_registration.h @@ -35,6 +35,7 @@ // Registers the chrome.tab.* functions we handle. #define REGISTER_TAB_API_FUNCTIONS() \ REGISTER_API_FUNCTION(GetTab); \ + REGISTER_API_FUNCTION(GetCurrentTab); \ REGISTER_API_FUNCTION(GetSelectedTab); \ REGISTER_API_FUNCTION(GetAllTabsInWindow); \ REGISTER_API_FUNCTION(CreateTab); \ diff --git a/ceee/ie/plugin/bho/browser_helper_object.cc b/ceee/ie/plugin/bho/browser_helper_object.cc index 21ad936..40109c7 100644 --- a/ceee/ie/plugin/bho/browser_helper_object.cc +++ b/ceee/ie/plugin/bho/browser_helper_object.cc @@ -557,7 +557,7 @@ bool BrowserHelperObject::EnsureTabId() { HRESULT hr = chrome_frame_host_->GetSessionId(&tab_id_); DCHECK(SUCCEEDED(hr)); if (hr == S_FALSE) { - // The server returned false, the session_id isn't available yet + // The server returned false, the session_id isn't available yet. return false; } @@ -1474,3 +1474,17 @@ void BrowserHelperObject::UnregisterSink(Sink* sink) { if (iter != sinks_.end()) sinks_.erase(iter); } + +STDMETHODIMP BrowserHelperObject::SetToolBandSessionId(long session_id) { + CeeeWindowHandle handle = reinterpret_cast<CeeeWindowHandle>(tab_window_); + HRESULT hr = broker_registrar_->SetTabToolBandIdForHandle(session_id, + handle); + 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 << + ")"; + return hr; +} diff --git a/ceee/ie/plugin/bho/browser_helper_object.h b/ceee/ie/plugin/bho/browser_helper_object.h index 074ee84..9bd689f 100644 --- a/ceee/ie/plugin/bho/browser_helper_object.h +++ b/ceee/ie/plugin/bho/browser_helper_object.h @@ -48,6 +48,7 @@ class ATL_NO_VTABLE BrowserHelperObject public IFrameEventHandlerHost, public IExtensionPortMessagingProvider, public IChromeFrameHostEvents, + public ICeeeBho, public ToolBandVisibility, public WebBrowserEventsSource { public: @@ -62,6 +63,7 @@ class ATL_NO_VTABLE BrowserHelperObject BEGIN_COM_MAP(BrowserHelperObject) COM_INTERFACE_ENTRY(IObjectWithSite) COM_INTERFACE_ENTRY(IPersist) + COM_INTERFACE_ENTRY(ICeeeBho) COM_INTERFACE_ENTRY_IID(IID_IFrameEventHandlerHost, IFrameEventHandlerHost) END_COM_MAP() @@ -168,6 +170,11 @@ class ATL_NO_VTABLE BrowserHelperObject virtual void UnregisterSink(Sink* sink); // @} + // @name ICeeeBho implementation + // @{ + STDMETHOD(SetToolBandSessionId)(long session_id); + // @} + protected: // Register proxy/stubs for executor interfaces. virtual HRESULT RegisterProxies(); diff --git a/ceee/ie/plugin/bho/browser_helper_object_unittest.cc b/ceee/ie/plugin/bho/browser_helper_object_unittest.cc index 9f4f7d8..53b2019 100644 --- a/ceee/ie/plugin/bho/browser_helper_object_unittest.cc +++ b/ceee/ie/plugin/bho/browser_helper_object_unittest.cc @@ -757,4 +757,22 @@ TEST_F(BrowserHelperObjectTest, IsHashChange) { ASSERT_HRESULT_SUCCEEDED(bho_with_site_->SetSite(NULL)); } +TEST_F(BrowserHelperObjectTest, SetToolBandSessionId) { + CreateSite(); + CreateBrowser(); + + ASSERT_HRESULT_SUCCEEDED(bho_with_site_->SetSite(site_keeper_)); + EXPECT_CALL(*(bho_->broker_), SetTabToolBandIdForHandle(kGoodTabId, + kGoodTabHandle)).WillOnce(Return(S_OK)); + EXPECT_EQ(S_OK, bho_->SetToolBandSessionId(kGoodTabId)); + + EXPECT_CALL(*(bho_->broker_), SetTabToolBandIdForHandle(kGoodTabId, + kGoodTabHandle)).WillOnce(Return(E_FAIL)); + EXPECT_EQ(E_FAIL, bho_->SetToolBandSessionId(kGoodTabId)); + + ExpectFireOnRemovedEvent(); + ExpectFireOnUnmappedEvent(); + ASSERT_HRESULT_SUCCEEDED(bho_with_site_->SetSite(NULL)); +} + } // namespace diff --git a/ceee/ie/plugin/toolband/tool_band.cc b/ceee/ie/plugin/toolband/tool_band.cc index d30c9a2..46f2bab 100644 --- a/ceee/ie/plugin/toolband/tool_band.cc +++ b/ceee/ie/plugin/toolband/tool_band.cc @@ -251,7 +251,7 @@ HRESULT ToolBand::Initialize(IUnknown* site) { // the BHO. Also required to get navigate2 notification. hr = service_provider->QueryService( SID_SWebBrowserApp, IID_IWebBrowser2, - reinterpret_cast<void**>(&web_browser_)); + reinterpret_cast<void**>(web_browser_.Receive())); DCHECK(SUCCEEDED(hr)); @@ -273,14 +273,14 @@ HRESULT ToolBand::Initialize(IUnknown* site) { } HRESULT ToolBand::InitializeAndShowWindow(IUnknown* site) { - CComPtr<IOleWindow> site_window; - HRESULT hr = site->QueryInterface(&site_window); + ScopedComPtr<IOleWindow> site_window; + HRESULT hr = site_window.QueryFrom(site); if (FAILED(hr)) { LOG(ERROR) << "Failed to get site window: " << com::LogHr(hr); return hr; } - DCHECK(NULL != site_window.p); + DCHECK(NULL != site_window.get()); hr = site_window->GetWindow(&parent_window_.m_hWnd); if (FAILED(hr)) { LOG(ERROR) << "Failed to get parent window handle: " << com::LogHr(hr); @@ -305,8 +305,8 @@ HRESULT ToolBand::Teardown() { if (IsWindow()) { // Teardown the ActiveX host window. CAxWindow host(m_hWnd); - CComPtr<IObjectWithSite> host_with_site; - HRESULT hr = host.QueryHost(&host_with_site); + ScopedComPtr<IObjectWithSite> host_with_site; + HRESULT hr = host.QueryHost(host_with_site.Receive()); if (SUCCEEDED(hr)) host_with_site->SetSite(NULL); @@ -335,8 +335,8 @@ LRESULT ToolBand::OnCreate(LPCREATESTRUCT lpCreateStruct) { GetUnknown()->AddRef(); // Create a host window instance. - CComPtr<IAxWinHostWindow> host; - HRESULT hr = CAxHostWindow::CreateInstance(&host); + ScopedComPtr<IAxWinHostWindow> host; + HRESULT hr = CAxHostWindow::CreateInstance(host.Receive()); if (FAILED(hr)) { LOG(ERROR) << "Failed to create ActiveX host window. " << com::LogHr(hr); return 1; @@ -539,9 +539,9 @@ STDMETHODIMP_(void) ToolBand::OnCfGetEnabledExtensionsComplete( current_height_ = 0; // Ask IE to reload all info for this toolband. - CComPtr<IOleCommandTarget> cmd_target; + ScopedComPtr<IOleCommandTarget> cmd_target; HRESULT hr = GetSite(IID_IOleCommandTarget, - reinterpret_cast<void**>(&cmd_target)); + reinterpret_cast<void**>(cmd_target.Receive())); if (SUCCEEDED(hr)) { CComVariant band_id(static_cast<int>(band_id_)); hr = cmd_target->Exec(&CGID_DeskBand, DBID_BANDINFOCHANGED, @@ -657,7 +657,10 @@ HRESULT ToolBand::EnsureBhoIsAvailable() { } #endif DVLOG(1) << "BHO already loaded"; - return S_OK; + if (existing_bho.vt != VT_UNKNOWN || existing_bho.punkVal == NULL) + return E_FAIL; + else + return SendSessionIdToBho(existing_bho.punkVal); } ScopedComPtr<IObjectWithSite> bho; @@ -675,12 +678,42 @@ HRESULT ToolBand::EnsureBhoIsAvailable() { } hr = web_browser_->PutProperty(bho_class_id_bstr, CComVariant(bho)); - LOG_IF(ERROR, FAILED(hr)) << "Assigning BHO to the web browser failed."; + if (FAILED(hr)) { + LOG(ERROR) << "Assigning BHO to the web browser failed." << com::LogHr(hr); + return hr; + } LOG_IF(INFO, SUCCEEDED(hr)) << "CEEE BHO has been created by the toolband."; - return hr; + + return SendSessionIdToBho(bho); } HRESULT ToolBand::CreateBhoInstance(IObjectWithSite** new_bho_instance) { DCHECK(new_bho_instance != NULL && *new_bho_instance == NULL); return BrowserHelperObject::CreateInstance(new_bho_instance); } + +HRESULT ToolBand::GetSessionId(int* session_id) { + if (chrome_frame_) { + ScopedComPtr<IChromeFrameInternal> chrome_frame_internal_; + chrome_frame_internal_.QueryFrom(chrome_frame_); + if (chrome_frame_internal_) { + return chrome_frame_internal_->getSessionId(session_id); + } + } + NOTREACHED(); + return E_UNEXPECTED; +} + +HRESULT ToolBand::SendSessionIdToBho(IUnknown* bho) { + // Now send the tool band's session ID to the BHO. + ScopedComPtr<ICeeeBho> ceee_bho; + HRESULT hr = ceee_bho.QueryFrom(bho); + if (SUCCEEDED(hr)) { + int session_id = 0; + hr = GetSessionId(&session_id); + if (SUCCEEDED(hr)) { + return ceee_bho->SetToolBandSessionId(session_id); + } + } + return E_FAIL; +} diff --git a/ceee/ie/plugin/toolband/tool_band.h b/ceee/ie/plugin/toolband/tool_band.h index e48dfd5..aad04c5 100644 --- a/ceee/ie/plugin/toolband/tool_band.h +++ b/ceee/ie/plugin/toolband/tool_band.h @@ -19,6 +19,7 @@ #include <string> #include "base/basictypes.h" +#include "base/scoped_comptr_win.h" #include "base/scoped_ptr.h" #include "base/win/rgs_helper.h" #include "ceee/ie/plugin/toolband/resource.h" @@ -181,6 +182,14 @@ class ATL_NO_VTABLE ToolBand : public CComObjectRootEx<CComSingleThreadModel>, // even though the bar will. HRESULT EnsureBhoIsAvailable(); + // Gets the session ID of the Chrome Frame instance associated with the tool + // band. + virtual HRESULT GetSessionId(int* session_id); + + // Sends the tool band's Chrome Frame session ID to the BHO, given the BHO as + // an IUnknown. + virtual HRESULT SendSessionIdToBho(IUnknown* bho); + private: // Initializes the toolband to the given site. // Called from SetSite. @@ -209,7 +218,7 @@ class ATL_NO_VTABLE ToolBand : public CComObjectRootEx<CComSingleThreadModel>, virtual HRESULT CreateBhoInstance(IObjectWithSite** new_bho_instance); // The web browser that initialized this toolband. - CComPtr<IWebBrowser2> web_browser_; + ScopedComPtr<IWebBrowser2> web_browser_; // Our parent window, yielded by our site's IOleWindow. CWindow parent_window_; // Our band id, provided by GetBandInfo. diff --git a/ceee/ie/plugin/toolband/tool_band_unittest.cc b/ceee/ie/plugin/toolband/tool_band_unittest.cc index 6dc184a..3ba9cad 100644 --- a/ceee/ie/plugin/toolband/tool_band_unittest.cc +++ b/ceee/ie/plugin/toolband/tool_band_unittest.cc @@ -24,10 +24,12 @@ namespace { using testing::_; +using testing::DoAll; using testing::GetConnectionCount; using testing::InstanceCountMixin; using testing::MockDispatchEx; using testing::Return; +using testing::SetArgumentPointee; using testing::StrEq; using testing::StrictMock; using testing::TestBrowser; @@ -38,13 +40,18 @@ class MockCeeeBho : public CComObjectRootEx<CComSingleThreadModel>, public InitializingCoClass<MockCeeeBho>, public IObjectWithSiteImpl<MockCeeeBho>, - public IPersistImpl<MockCeeeBho> { + public IPersistImpl<MockCeeeBho>, + public ICeeeBho { public: BEGIN_COM_MAP(MockCeeeBho) COM_INTERFACE_ENTRY(IObjectWithSite) COM_INTERFACE_ENTRY(IPersist) + COM_INTERFACE_ENTRY(ICeeeBho) END_COM_MAP() + MOCK_METHOD1_WITH_CALLTYPE(__stdcall, SetToolBandSessionId, + HRESULT(long session_id)); + static const CLSID& WINAPI GetObjectCLSID() { // Required to make IPersistImpl work. return BrowserHelperObject::GetObjectCLSID(); @@ -63,6 +70,9 @@ class TestingToolBand public InstanceCountMixin<TestingToolBand>, public InitializingCoClass<TestingToolBand> { public: + MOCK_METHOD1(SendSessionIdToBho, HRESULT(IUnknown*)); + MOCK_METHOD1(GetSessionId, HRESULT(int*)); + HRESULT Initialize(TestingToolBand** self) { bho_counter_ = 0; *self = this; @@ -73,6 +83,10 @@ class TestingToolBand return EnsureBhoIsAvailable(); } + HRESULT CallSendSessionIdToBho(IUnknown* bho) { + return ToolBand::SendSessionIdToBho(bho); + } + int bho_counter_; private: virtual HRESULT InitializeAndShowWindow(IUnknown* site) { @@ -443,6 +457,7 @@ TEST_F(BhoInToolBandTest, EnsureBhoCreatedWhenNeeded) { CComVariant variant_with_bho(mocked_bho_with_site); EXPECT_CALL(*browser_, GetProperty(StrEq(bho_class_id_text_), _)). WillOnce(SetArg1Variant(&variant_with_bho)); + EXPECT_CALL(*tool_band_, SendSessionIdToBho(_)).WillOnce(Return(S_OK)); ASSERT_HRESULT_SUCCEEDED(tool_band_->CallEnsureBhoIsAvailable()); ASSERT_TRUE(mocked_bho->m_spUnkSite == NULL); @@ -455,6 +470,7 @@ TEST_F(BhoInToolBandTest, EnsureBhoCreatedWhenNeeded) { // New bho should be dropped into the dropbox. EXPECT_CALL(*browser_, PutProperty(StrEq(bho_class_id_text_), _)). WillOnce(GetArg1Variant(&bho_dropbox)); + EXPECT_CALL(*tool_band_, SendSessionIdToBho(_)).WillOnce(Return(S_OK)); ASSERT_HRESULT_SUCCEEDED(tool_band_->CallEnsureBhoIsAvailable()); ASSERT_EQ(bho_dropbox.vt, VT_UNKNOWN); ASSERT_TRUE(bho_dropbox.punkVal != NULL); @@ -486,4 +502,24 @@ TEST_F(BhoInToolBandTest, BhoCreationErrorHandling) { ASSERT_EQ(tool_band_->bho_counter_, 0); } +TEST_F(ToolBandTest, SendSessionIdToBho) { + MockCeeeBho* mock_bho; + CComPtr<IObjectWithSite> mock_bho_with_site; + ASSERT_HRESULT_SUCCEEDED( + MockCeeeBho::CreateInitialized(&mock_bho, &mock_bho_with_site)); + EXPECT_CALL(*tool_band_, GetSessionId(_)).WillOnce(DoAll( + SetArgumentPointee<0>(5), Return(S_OK))); + EXPECT_CALL(*mock_bho, SetToolBandSessionId(5)).WillOnce(Return(S_OK)); + EXPECT_EQ(S_OK, tool_band_->CallSendSessionIdToBho(mock_bho_with_site)); + + // Test error handling. + EXPECT_CALL(*tool_band_, GetSessionId(_)).WillOnce(Return(E_FAIL)); + EXPECT_EQ(E_FAIL, tool_band_->CallSendSessionIdToBho(mock_bho_with_site)); + + EXPECT_CALL(*tool_band_, GetSessionId(_)).WillOnce(DoAll( + SetArgumentPointee<0>(6), Return(S_OK))); + EXPECT_CALL(*mock_bho, SetToolBandSessionId(6)).WillOnce(Return(E_FAIL)); + EXPECT_EQ(E_FAIL, tool_band_->CallSendSessionIdToBho(mock_bho_with_site)); +} + } // namespace diff --git a/ceee/ie/plugin/toolband/toolband.idl b/ceee/ie/plugin/toolband/toolband.idl index f85fff9..1a2eb59 100644 --- a/ceee/ie/plugin/toolband/toolband.idl +++ b/ceee/ie/plugin/toolband/toolband.idl @@ -22,7 +22,7 @@ import "ocidl.idl"; pointer_default(unique) ] // Native interface to content scripts. This interface is exposed to -// ceee_boostrap.js, and is used to satisfy the same contract as +// ceee_bootstrap.js, and is used to satisfy the same contract as // Chrome's native functions declared in event_bindings.js and // renderer_extension_bindings.js. Additionally this interface exposes // properties for the dispatch functions declared in the same files. @@ -108,6 +108,22 @@ typedef struct { [ object, + uuid(58A708CC-2F8F-4d83-B1EF-54AA79FE0522), + nonextensible, + helpstring("ICeeeBho Interface"), + pointer_default(unique), + oleautomation +] +// Object provided to the tool band to pass information to the BHO. +interface ICeeeBho : IUnknown { + // Sends the tool band's Chrome Frame session ID to the BHO. + // + // @param session_id The session ID of the toolband associated with the BHO. + HRESULT SetToolBandSessionId([in] long session_id); +}; + +[ + object, uuid(66CDB7E2-B326-493e-B469-16234426C89B), nonextensible, helpstring("ICeeeWindowExecutor Interface"), @@ -326,7 +342,7 @@ interface ICeeeExecutorCreator : IUnknown { }; [ - uuid(7C09079D-F9CB-4E9E-9293-D224B071D8BA), + uuid(11230B2D-C7FF-46b4-B464-C46E9B0E551F), version(1.0), helpstring("Google CEEE 1.0 Type Library") ] @@ -335,15 +351,19 @@ library ToolbandLib { // include type info in .tlb interface ICEEEContentScriptNativeApi; + interface ICeeeBho; interface ICeeeTabExecutor; interface ICeeeWindowExecutor; + interface ICeeeCookieExecutor; + interface ICeeeInfobarExecutor; [ - uuid(E49EBDB7-CEC9-4014-A5F5-8D3C8F5997DC), + uuid(210F4C27-5109-4471-A903-4AB7FB90AE1F), helpstring("BrowserHelperObject Class") ] coclass BrowserHelperObject { [default] interface IUnknown; + interface ICeeeBho; }; [ uuid(2F1A2D6B-55F6-4B63-8C37-F698D28FDC2B), diff --git a/ceee/ie/testing/mock_broker_and_friends.h b/ceee/ie/testing/mock_broker_and_friends.h index 29ab078..33df433 100644 --- a/ceee/ie/testing/mock_broker_and_friends.h +++ b/ceee/ie/testing/mock_broker_and_friends.h @@ -38,6 +38,8 @@ class MockBrokerRegistrarImpl : public ICeeeBrokerRegistrar { HRESULT(long, IUnknown*)); MOCK_METHOD2_WITH_CALLTYPE(__stdcall, SetTabIdForHandle, HRESULT(long, CeeeWindowHandle)); + MOCK_METHOD2_WITH_CALLTYPE(__stdcall, SetTabToolBandIdForHandle, + HRESULT(long, CeeeWindowHandle)); }; class MockBroker @@ -240,6 +242,7 @@ class MockApiDispatcher : public ApiDispatcher { MOCK_METHOD2(FireEvent, void(BSTR event_name, BSTR event_args)); MOCK_CONST_METHOD1(GetTabHandleFromId, HWND(int)); + MOCK_CONST_METHOD1(GetTabHandleFromToolBandId, HWND(int)); MOCK_CONST_METHOD1(GetWindowHandleFromId, HWND(int)); MOCK_CONST_METHOD1(GetTabIdFromHandle, int(HWND)); MOCK_CONST_METHOD1(GetWindowIdFromHandle, int(HWND)); |