arc: Add InstanceHelper::GetInstanceForMethod()

components/arc/ime/arc_ime_bridge_impl.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/arc/ime/arc_ime_bridge_impl.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/arc/arc_bridge_service.h"
 #include "ui/base/ime/composition_text.h"
 #include "ui/base/ime/text_input_type.h"
 #include "ui/gfx/geometry/rect.h"
 
 namespace arc {
 namespace {
 
-constexpr int kMinVersionForOnKeyboardsBoundsChanging = 3;
-constexpr int kMinVersionForExtendSelectionAndDelete = 4;
+constexpr uint32_t kMinInstanceVersion = 0;

[Yusuke Sato, 2016/09/16 23:58:48]

You didn't introduce version zero checks for BT, so this is a bit inconsistent,
I think.

[Luis Héctor Chávez, 2016/09/17 00:30:53]

Added version zero checks for BT.

[Yusuke Sato, 2016/09/17 01:38:56]

Ok, so I think you're saying that we should always use GetInstanceForVersion()
even when the minimum required version is 0, correct?

Assuming so, that seems like a good practice as using the raw instance()
function is more error prone.

+constexpr uint32_t kMinVersionForOnKeyboardsBoundsChanging = 3;
+constexpr uint32_t kMinVersionForExtendSelectionAndDelete = 4;
 
 ui::TextInputType ConvertTextInputType(arc::mojom::TextInputType ipc_type) {
   // The two enum types are similar, but intentionally made not identical.
   // We cannot force them to be in sync. If we do, updates in ui::TextInputType
   // must always be propagated to the arc::mojom::TextInputType mojo definition
   // in
   // ARC container side, which is in a different repository than Chromium.
   // We don't want such dependency.
   //
   // That's why we need a lengthy switch statement instead of static_cast
@@ skipping 56 matching lines @@
   bridge_service_->ime()->RemoveObserver(this);
 }
 
 void ArcImeBridgeImpl::OnInstanceReady() {
   bridge_service_->ime()->instance()->Init(
       binding_.CreateInterfacePtrAndBind());
 }
 
 void ArcImeBridgeImpl::SendSetCompositionText(
     const ui::CompositionText& composition) {
-  mojom::ImeInstance* ime_instance = bridge_service_->ime()->instance();
-  if (!ime_instance) {
-    LOG(ERROR) << "ArcImeInstance method called before being ready.";
+  auto* ime_instance = bridge_service_->ime()->GetInstanceForVersion(
+      kMinInstanceVersion, "SetCompositionText");
+  if (!ime_instance)
     return;
-  }
 
   ime_instance->SetCompositionText(base::UTF16ToUTF8(composition.text),
                                    ConvertSegments(composition));
 }
 
 void ArcImeBridgeImpl::SendConfirmCompositionText() {
-  mojom::ImeInstance* ime_instance = bridge_service_->ime()->instance();
-  if (!ime_instance) {
-    LOG(ERROR) << "ArcImeInstance method called before being ready.";
+  auto* ime_instance = bridge_service_->ime()->GetInstanceForVersion(
+      kMinInstanceVersion, "ConfirmCompositionText");
+  if (!ime_instance)
     return;
-  }
 
   ime_instance->ConfirmCompositionText();
 }
 
 void ArcImeBridgeImpl::SendInsertText(const base::string16& text) {
-  mojom::ImeInstance* ime_instance = bridge_service_->ime()->instance();
-  if (!ime_instance) {
-    LOG(ERROR) << "ArcImeInstance method called before being ready.";
+  auto* ime_instance = bridge_service_->ime()->GetInstanceForVersion(
+      kMinInstanceVersion, "SendInsertText");

[Yusuke Sato, 2016/09/16 23:58:48]

s/Send// ?

[Luis Héctor Chávez, 2016/09/17 00:30:53]

Done.

+  if (!ime_instance)
     return;
-  }
 
   ime_instance->InsertText(base::UTF16ToUTF8(text));
 }
 
 void ArcImeBridgeImpl::SendOnKeyboardBoundsChanging(
     const gfx::Rect& new_bounds) {
-  mojom::ImeInstance* ime_instance = bridge_service_->ime()->instance();
-  if (!ime_instance) {
-    LOG(ERROR) << "ArcImeInstance method called before being ready.";
+  auto* ime_instance = bridge_service_->ime()->GetInstanceForVersion(
+      kMinVersionForOnKeyboardsBoundsChanging, "OnKeyboardBoundsChanging");
+  if (!ime_instance)
     return;
-  }
-  if (bridge_service_->ime()->version() <
-      kMinVersionForOnKeyboardsBoundsChanging) {
-    LOG(ERROR) << "ArcImeInstance is too old for OnKeyboardsBoundsChanging.";
-    return;
-  }
 
   ime_instance->OnKeyboardBoundsChanging(new_bounds);
 }
 
 void ArcImeBridgeImpl::SendExtendSelectionAndDelete(
     size_t before, size_t after) {
-  mojom::ImeInstance* ime_instance = bridge_service_->ime()->instance();
-  if (!ime_instance) {
-    LOG(ERROR) << "ArcImeInstance method called before being ready.";
+  auto* ime_instance = bridge_service_->ime()->GetInstanceForVersion(
+      kMinVersionForExtendSelectionAndDelete, "ExtendSelectionAndDelete");
+  if (!ime_instance)
     return;
-  }
-  if (bridge_service_->ime()->version() <
-      kMinVersionForExtendSelectionAndDelete) {
-    LOG(ERROR) << "ArcImeInstance is too old for ExtendSelectionAndDelete.";
-    return;
-  }
 
   ime_instance->ExtendSelectionAndDelete(before, after);
 }
 
 void ArcImeBridgeImpl::OnTextInputTypeChanged(arc::mojom::TextInputType type) {
   delegate_->OnTextInputTypeChanged(ConvertTextInputType(type));
 }
 
 void ArcImeBridgeImpl::OnCursorRectChanged(arc::mojom::CursorRectPtr rect) {
   delegate_->OnCursorRectChanged(gfx::Rect(rect->left, rect->top,
                                            rect->right - rect->left,
                                            rect->bottom - rect->top));
 }
 
 void ArcImeBridgeImpl::OnCancelComposition() {
   delegate_->OnCancelComposition();
 }
 
 void ArcImeBridgeImpl::ShowImeIfNeeded() {
   delegate_->ShowImeIfNeeded();
 }
 
 }  // namespace arc