Redesign: Remove TsfEventRouter interface.

ui/base/ime/win/tsf_event_router.cc

Index: ui/base/ime/win/tsf_event_router.cc
diff --git a/ui/base/ime/win/tsf_event_router.cc b/ui/base/ime/win/tsf_event_router.cc
index b26b27d71bb5cf692de3fb88aa9a577ebc143a02..9071342102c0fa9b7d750a00cf546881bfffc929 100644
--- a/ui/base/ime/win/tsf_event_router.cc
+++ b/ui/base/ime/win/tsf_event_router.cc
@@ -11,266 +11,260 @@
 #include "base/bind.h"
 #include "base/win/scoped_comptr.h"
 #include "base/win/metro.h"
+#include "ui/base/win/atl_module.h"
 
 namespace ui {
 

[Peter Kasting, 2012/10/26 02:31:57]

Tiny nit: I suggest one more blank line above each of the two divider comments,
just to set each section off more from the stuff above it.

[Seigo Nonaka, 2012/10/26 02:40:27]

Done.

-class TsfEventRouterImpl : public TsfEventRouter,
-                           public ITfUIElementSink,
-                           public ITfTextEditSink {
+// TsfEventRouter::TsfEventRouterDelegate  ------------------------------------
+// The implementation class of ITfUIElementSink, whose member functions will be

[Peter Kasting, 2012/10/26 02:31:57]

Nit: Add blank line here

[Seigo Nonaka, 2012/10/26 02:40:27]

Done.

+// called back by TSF when the UI element status is changed, for example when
+// the candidate window is opened or closed. This class also implements
+// ITfTextEditSink, whose member function is called back by TSF when the text
+// editting session is finished.
+class ATL_NO_VTABLE TsfEventRouter::TsfEventRouterDelegate
+    : public ATL::CComObjectRootEx<CComSingleThreadModel>,
+      public ITfUIElementSink,
+      public ITfTextEditSink {
  public:
-  TsfEventRouterImpl()
-      : observer_(NULL),
-        context_source_cookie_(TF_INVALID_COOKIE),
-        ui_source_cookie_(TF_INVALID_COOKIE),
-        ref_count_(0) {}
+  BEGIN_COM_MAP(TsfEventRouterDelegate)
+    COM_INTERFACE_ENTRY(ITfUIElementSink)
+    COM_INTERFACE_ENTRY(ITfTextEditSink)
+  END_COM_MAP()
 
-  virtual ~TsfEventRouterImpl() {}
+  TsfEventRouterDelegate();
+  ~TsfEventRouterDelegate();
 
-  // ITfTextEditSink override.
-  virtual ULONG STDMETHODCALLTYPE AddRef() OVERRIDE {
-    return InterlockedIncrement(&ref_count_);
-  }
+  // ITfTextEditSink:
+  STDMETHOD_(HRESULT, OnEndEdit)(ITfContext* context,
+                                 TfEditCookie read_only_cookie,
+                                 ITfEditRecord* edit_record) OVERRIDE;
 
-  // ITfTextEditSink override.
-  virtual ULONG STDMETHODCALLTYPE Release() OVERRIDE {
-    const LONG count = InterlockedDecrement(&ref_count_);
-    if (!count) {
-      delete this;
-      return 0;
-    }
-    return static_cast<ULONG>(count);
-  }
+  // ITfUiElementSink:
+  STDMETHOD_(HRESULT, BeginUIElement)(DWORD element_id, BOOL* is_show) OVERRIDE;
+  STDMETHOD_(HRESULT, UpdateUIElement)(DWORD element_id) OVERRIDE;
+  STDMETHOD_(HRESULT, EndUIElement)(DWORD element_id) OVERRIDE;
 
-  // ITfTextEditSink override.
-  virtual STDMETHODIMP QueryInterface(REFIID iid, void** result) OVERRIDE {
-    if (!result)
-      return E_INVALIDARG;
-    if (iid == IID_IUnknown || iid == IID_ITfTextEditSink) {
-      *result = static_cast<ITfTextEditSink*>(this);
-    } else if (iid == IID_ITfUIElementSink) {
-      *result = static_cast<ITfUIElementSink*>(this);
-    } else {
-      *result = NULL;
-      return E_NOINTERFACE;
-    }
-    AddRef();
-    return S_OK;
-  }
+  // Sets |thread_manager| to be monitored. |thread_manager| can be NULL.
+  void SetManager(ITfThreadMgr* thread_manager);

[Peter Kasting, 2012/10/26 02:31:57]

Nit: I suggest using the same parameter name in this function and
TsfEventRouter::SetManager().  (I don't care which one you use.)

[Seigo Nonaka, 2012/10/26 02:40:27]

Done.

 
-  // ITfTextEditSink override.
-  virtual STDMETHODIMP OnEndEdit(ITfContext* context,
-                                 TfEditCookie read_only_cookie,
-                                 ITfEditRecord* edit_record) OVERRIDE {
-    if (!edit_record || !context)
-      return E_INVALIDARG;
-    if (!observer_)
-      return S_OK;
-
-    // |edit_record| can be used to obtain updated ranges in terms of text
-    // contents and/or text attributes. Here we are interested only in text
-    // update so we use TF_GTP_INCL_TEXT and check if there is any range which
-    // contains updated text.
-    base::win::ScopedComPtr<IEnumTfRanges> ranges;
-    if (FAILED(edit_record->GetTextAndPropertyUpdates(TF_GTP_INCL_TEXT,
-                                                      NULL,
-                                                      0,
-                                                      ranges.Receive()))) {
-       return S_OK;  // don't care about failures.
-    }
-
-    ULONG fetched_count = 0;
-    base::win::ScopedComPtr<ITfRange> range;
-    if (FAILED(ranges->Next(1, range.Receive(), &fetched_count)))
-      return S_OK;  // don't care about failures.
-
-    // |fetched_count| != 0 means there is at least one range that contains
-    // updated texts.
-    if (fetched_count != 0)
-      observer_->OnTextUpdated();
-    return S_OK;
-  }
+  // Returns true if the IME is composing text.
+  bool IsImeComposing();
 
-  // ITfUiElementSink override.
-  virtual STDMETHODIMP BeginUIElement(DWORD element_id, BOOL* is_show) {
-    if (is_show)
-      *is_show = TRUE;  // without this the UI element will not be shown.
+  // Sets |router| to be forwarded TSF-related events.
+  void SetRouter(TsfEventRouter* router);
 
-    if (!IsCandidateWindowInternal(element_id))
-      return S_OK;
+ private:
+  // Returns true if the given |context| is composing.
+  static bool IsImeComposingInternal(ITfContext* context);
 
-    std::pair<std::set<DWORD>::iterator, bool> insert_result =
-        open_candidate_window_ids_.insert(element_id);
+  // Returns true if the given |element_id| represents the candidate window.
+  bool IsCandidateWindowInternal(DWORD element_id);
 
-    if (!observer_)
-      return S_OK;
+  // A context associated with this class.
+  base::win::ScopedComPtr<ITfContext> context_;
 
-    // Don't call if |element_id| is already handled.
-    if (!insert_result.second)
-      return S_OK;
+  // The ITfSource associated with |context_|.
+  base::win::ScopedComPtr<ITfSource> context_source_;
 
-    observer_->OnCandidateWindowCountChanged(open_candidate_window_ids_.size());
+  // The cookie for |context_source_|.
+  DWORD context_source_cookie_;
 
-    return S_OK;
-  }
+  // A UIElementMgr associated with this class.
+  base::win::ScopedComPtr<ITfUIElementMgr> ui_element_manager_;
 
-  // ITfUiElementSink override.
-  virtual STDMETHODIMP UpdateUIElement(DWORD element_id) {
-    return S_OK;
-  }
+  // The ITfSouce associated with |ui_element_manager_|.
+  base::win::ScopedComPtr<ITfSource> ui_source_;
 
-  // ITfUiElementSink override.
-  virtual STDMETHODIMP EndUIElement(DWORD element_id) {
-    if (open_candidate_window_ids_.erase(element_id) == 0)
-      return S_OK;
+  // The set of currently opened candidate window ids.
+  std::set<DWORD> open_candidate_window_ids_;
 
-    if (!observer_)
-      return S_OK;
+  // The cookie for |ui_source_|.
+  DWORD ui_source_cookie_;
 
-    observer_->OnCandidateWindowCountChanged(open_candidate_window_ids_.size());
+  TsfEventRouter* router_;
 
-    return S_OK;
-  }
+  DISALLOW_COPY_AND_ASSIGN(TsfEventRouterDelegate);
+};
 
-  // TsfEventRouter override.
-  virtual void SetManager(ITfThreadMgr* manager, Observer* observer) OVERRIDE {
-    EnsureDeassociated();
-    if (manager && observer) {
-      Associate(manager, observer);
-    }
-  }
+TsfEventRouter::TsfEventRouterDelegate::TsfEventRouterDelegate()
+    : context_source_cookie_(TF_INVALID_COOKIE),
+      ui_source_cookie_(TF_INVALID_COOKIE),
+      router_(NULL) {
+}
 
-  // TsfEventRouter override.
-  virtual bool IsImeComposing() OVERRIDE {
-    DCHECK(base::win::IsTsfAwareRequired())
-        << "Do not call without TSF environment.";
-    if (!context_)
-      return false;
-    return IsImeComposingInternal(context_);
-  }
+TsfEventRouter::TsfEventRouterDelegate::~TsfEventRouterDelegate() {}
 
- private:
-  // Returns true if the given |context| is in composing.
-  static bool IsImeComposingInternal(ITfContext* context) {
-    DCHECK(base::win::IsTsfAwareRequired())
-        << "Do not call without TSF environment.";
-    DCHECK(context);
-    base::win::ScopedComPtr<ITfContextComposition> context_composition;
-    if (FAILED(context_composition.QueryFrom(context)))
-      return false;
-    base::win::ScopedComPtr<IEnumITfCompositionView> enum_composition_view;
-    if (FAILED(context_composition->EnumCompositions(
-            enum_composition_view.Receive())))
-      return false;
-    base::win::ScopedComPtr<ITfCompositionView> composition_view;
-    return enum_composition_view->Next(1, composition_view.Receive(),
-               NULL) == S_OK;
-  }
+void TsfEventRouter::TsfEventRouterDelegate::SetRouter(TsfEventRouter* router) {
+  router_ = router;
+}
 
-  // Returns true if the given |element_id| represents candidate window.
-  bool IsCandidateWindowInternal(DWORD element_id) {
-    DCHECK(ui_element_manager_.get());
-    base::win::ScopedComPtr<ITfUIElement> ui_element;
-    if (FAILED(ui_element_manager_->GetUIElement(element_id,
-                                                 ui_element.Receive()))) {
-      return false;
-    }
-    base::win::ScopedComPtr<ITfCandidateListUIElement>
-        candidate_list_ui_element;
-    return SUCCEEDED(candidate_list_ui_element.QueryFrom(ui_element));
-  }
+STDMETHODIMP TsfEventRouter::TsfEventRouterDelegate::OnEndEdit(
+    ITfContext* context,
+    TfEditCookie read_only_cookie,
+    ITfEditRecord* edit_record) {
+  if (!edit_record || !context)
+    return E_INVALIDARG;
+  if (!router_)
+    return S_OK;
 
-  // Associates this class with specified |manager|.
-  void Associate(ITfThreadMgr* thread_manager, Observer* observer) {
-    DCHECK(base::win::IsTsfAwareRequired())
-        << "Do not call without TSF environment.";
-    DCHECK(thread_manager);
-
-    base::win::ScopedComPtr<ITfDocumentMgr> document_manager;
-    if (FAILED(thread_manager->GetFocus(document_manager.Receive())))
-      return;
-
-    if (FAILED(document_manager->GetBase(context_.Receive())))
-      return;
-    if (FAILED(context_source_.QueryFrom(context_)))
-      return;
-    context_source_->AdviseSink(IID_ITfTextEditSink,
-                                static_cast<ITfTextEditSink*>(this),
-                                &context_source_cookie_);
-
-    if (FAILED(ui_element_manager_.QueryFrom(thread_manager)))
-      return;
-    if (FAILED(ui_source_.QueryFrom(ui_element_manager_)))
-      return;
-    ui_source_->AdviseSink(IID_ITfUIElementSink,
-                           static_cast<ITfUIElementSink*>(this),
-                           &ui_source_cookie_);
-    observer_ = observer;
-  }
+  // |edit_record| can be used to obtain updated ranges in terms of text
+  // contents and/or text attributes. Here we are interested only in text update
+  // so we use TF_GTP_INCL_TEXT and check if there is any range which contains
+  // updated text.
+  base::win::ScopedComPtr<IEnumTfRanges> ranges;
+  if (FAILED(edit_record->GetTextAndPropertyUpdates(TF_GTP_INCL_TEXT, NULL, 0,
+                                                    ranges.Receive())))
+     return S_OK;  // Don't care about failures.
+
+  ULONG fetched_count = 0;
+  base::win::ScopedComPtr<ITfRange> range;
+  if (FAILED(ranges->Next(1, range.Receive(), &fetched_count)))
+    return S_OK;  // Don't care about failures.
+
+  // |fetched_count| != 0 means there is at least one range that contains
+  // updated text.
+  if (fetched_count != 0)
+    router_->OnTextUpdated();
+  return S_OK;
+}
 
-  // Resets the association, this function is safe to call if there is no
-  // associated thread manager.
-  void EnsureDeassociated() {
-    DCHECK(base::win::IsTsfAwareRequired())
-        << "Do not call without TSF environment.";
+STDMETHODIMP TsfEventRouter::TsfEventRouterDelegate::BeginUIElement(
+    DWORD element_id,
+    BOOL* is_show) {
+  if (is_show)
+    *is_show = TRUE;  // Without this the UI element will not be shown.
 
-    context_.Release();
+  if (!IsCandidateWindowInternal(element_id))
+    return S_OK;
 
-    if (context_source_) {
-      context_source_->UnadviseSink(context_source_cookie_);
-      context_source_.Release();
-    }
-    context_source_cookie_ = TF_INVALID_COOKIE;
+  std::pair<std::set<DWORD>::iterator, bool> insert_result =
+      open_candidate_window_ids_.insert(element_id);
+  // Don't call if |router_| is null or |element_id| is already handled.
+  if (router_ && insert_result.second)
+    router_->OnCandidateWindowCountChanged(open_candidate_window_ids_.size());
 
-    ui_element_manager_.Release();
-    if (ui_source_) {
-      ui_source_->UnadviseSink(ui_source_cookie_);
-      ui_source_.Release();
-    }
-    ui_source_cookie_ = TF_INVALID_COOKIE;
+  return S_OK;
+}
 
-    observer_ = NULL;
-  }
+STDMETHODIMP TsfEventRouter::TsfEventRouterDelegate::UpdateUIElement(
+    DWORD element_id) {
+  return S_OK;
+}
 
-  Observer* observer_;
+STDMETHODIMP TsfEventRouter::TsfEventRouterDelegate::EndUIElement(
+    DWORD element_id) {
+  if ((open_candidate_window_ids_.erase(element_id) != 0) && router_)
+    router_->OnCandidateWindowCountChanged(open_candidate_window_ids_.size());
+  return S_OK;
+}
 
-  // A context associated with this class.
-  base::win::ScopedComPtr<ITfContext> context_;
+void TsfEventRouter::TsfEventRouterDelegate::SetManager(
+    ITfThreadMgr* thread_manager) {
+  context_.Release();
 
-  // The ITfSource associated with |context_|.
-  base::win::ScopedComPtr<ITfSource> context_source_;
+  if (context_source_) {
+    context_source_->UnadviseSink(context_source_cookie_);
+    context_source_.Release();
+  }
+  context_source_cookie_ = TF_INVALID_COOKIE;
 
-  // The cookie for |context_source_|.
-  DWORD context_source_cookie_;
+  ui_element_manager_.Release();
+  if (ui_source_) {
+    ui_source_->UnadviseSink(ui_source_cookie_);
+    ui_source_.Release();
+  }
+  ui_source_cookie_ = TF_INVALID_COOKIE;
+
+  if (!thread_manager)
+    return;
+
+  base::win::ScopedComPtr<ITfDocumentMgr> document_manager;
+  if (FAILED(thread_manager->GetFocus(document_manager.Receive())) ||
+      FAILED(document_manager->GetBase(context_.Receive())) ||
+      FAILED(context_source_.QueryFrom(context_)))
+    return;
+  context_source_->AdviseSink(IID_ITfTextEditSink,
+                              static_cast<ITfTextEditSink*>(this),
+                              &context_source_cookie_);
+
+  if (FAILED(ui_element_manager_.QueryFrom(thread_manager)) ||
+      FAILED(ui_source_.QueryFrom(ui_element_manager_)))
+    return;
+  ui_source_->AdviseSink(IID_ITfUIElementSink,
+                         static_cast<ITfUIElementSink*>(this),
+                         &ui_source_cookie_);
+}
 
-  // A UiElementMgr associated with this class.
-  base::win::ScopedComPtr<ITfUIElementMgr> ui_element_manager_;
+bool TsfEventRouter::TsfEventRouterDelegate::IsImeComposing() {
+  return context_ && IsImeComposingInternal(context_);
+}
 
-  // The ITfSouce associated with |ui_element_manager_|.
-  base::win::ScopedComPtr<ITfSource> ui_source_;
+// static
+bool TsfEventRouter::TsfEventRouterDelegate::IsImeComposingInternal(
+    ITfContext* context) {
+  DCHECK(context);
+  base::win::ScopedComPtr<ITfContextComposition> context_composition;
+  if (FAILED(context_composition.QueryFrom(context)))
+    return false;
+  base::win::ScopedComPtr<IEnumITfCompositionView> enum_composition_view;
+  if (FAILED(context_composition->EnumCompositions(
+      enum_composition_view.Receive())))
+    return false;
+  base::win::ScopedComPtr<ITfCompositionView> composition_view;
+  return enum_composition_view->Next(1, composition_view.Receive(),
+                                     NULL) == S_OK;
+}
 
-  // The cookie for |ui_source_|.
-  DWORD ui_source_cookie_;
+bool TsfEventRouter::TsfEventRouterDelegate::IsCandidateWindowInternal(
+    DWORD element_id) {
+  DCHECK(ui_element_manager_.get());
+  base::win::ScopedComPtr<ITfUIElement> ui_element;
+  if (FAILED(ui_element_manager_->GetUIElement(element_id,
+                                               ui_element.Receive())))
+    return false;
+  base::win::ScopedComPtr<ITfCandidateListUIElement> candidate_list_ui_element;
+  return SUCCEEDED(candidate_list_ui_element.QueryFrom(ui_element));
+}
 
-  // The set of currently opend candidate window ids.
-  std::set<DWORD> open_candidate_window_ids_;
 
-  // The reference count of this instance.
-  volatile LONG ref_count_;
+// TsfEventRouter  ------------------------------------------------------------
+
+TsfEventRouter::TsfEventRouter(TsfEventRouterObserver* observer)
+    : observer_(observer),
+      delegate_(NULL) {
+  DCHECK(base::win::IsTsfAwareRequired())
+      << "Do not use TsfEventRouter without TSF environment.";
+  CComObject<TsfEventRouterDelegate>* delegate;
+  ui::win::CreateATLModuleIfNeeded();
+  if (SUCCEEDED(CComObject<TsfEventRouterDelegate>::CreateInstance(
+      &delegate))) {
+    delegate->AddRef();
+    delegate_.Attach(delegate_);
+    delegate_->SetRouter(this);
+  }
+}
 
-  DISALLOW_COPY_AND_ASSIGN(TsfEventRouterImpl);
-};
+TsfEventRouter::~TsfEventRouter() {
+  if (delegate_) {
+    delegate_->SetManager(NULL);
+    delegate_->SetRouter(NULL);
+  }
+}
 
-///////////////////////////////////////////////////////////////////////////////
-// TsfEventRouter
+void TsfEventRouter::SetManager(ITfThreadMgr* manager) {
+  delegate_->SetManager(manager);
+}
 
-TsfEventRouter::TsfEventRouter() {
+bool TsfEventRouter::IsImeComposing() {
+  return delegate_->IsImeComposing();
 }
 
-TsfEventRouter::~TsfEventRouter() {
+void TsfEventRouter::OnTextUpdated() {
+  observer_->OnTextUpdated();
 }
 
-TsfEventRouter* TsfEventRouter::Create() {
-  return new TsfEventRouterImpl();
+void TsfEventRouter::OnCandidateWindowCountChanged(size_t window_count) {
+  observer_->OnCandidateWindowCountChanged(window_count);
 }
 
 }  // namespace ui