Fix highly confusing misnomer ComponentExtensionIMEManagerImpl.

Despite what the current name implies, ComponentExtensionIMEManagerImpl
is NOT an implementation of ComponentExtensionIMEManager interface
(that's actually a concrete class, only ever subclassed by its mock).
This is extremely confusing in terms of readability.

In fact, ComponentExtensionIMEManagerImpl [sic] is an implementation
of ComponentExtensionIMEManagerDelegate interface. An instance of the
latter is dependency-injected for use by ComponentExtensionIMEManager
itself. The already confusing misnomer is thus exacerbated, as both
ComponentExtensionIMEManager and ComponentExtensionIMEManagerImpl [sic]
tend to appear together while bearing no inheritance relationship.

In this patch, ComponentExtensionIMEManagerImpl [sic] is renamed to
ComponentExtensionIMEManagerDelegateImpl to reflect what it really is.
Descriptiveness and conciseness improvements are out of scope.

Also:
- Extract ComponentExtensionIMEManagerDelegate to its own .h file.
- Fix pre-existing std::unique_ptr usage as flagged by presubmit errors.
- Replace pre-existing NULL with nullptr as flagged by presubmit errors.

Bug: 1134465
Change-Id: If36b7d4c17ee19cdfc773c22ad4f384779b2a4fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2440381
Auto-Submit: Bao-Duy Tran <tranbaoduy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Keith Lee <keithlee@chromium.org>
Reviewed-by: David Vallet <dvallet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816644}

chrome/browser/chromeos/BUILD.gn

@@ -1411,8 +1411,8 @@ source_set("chromeos") {
     "input_method/candidate_window_controller.h",
     "input_method/candidate_window_controller_impl.cc",
     "input_method/candidate_window_controller_impl.h",
-    "input_method/component_extension_ime_manager_impl.cc",
-    "input_method/component_extension_ime_manager_impl.h",
+    "input_method/component_extension_ime_manager_delegate_impl.cc",
+    "input_method/component_extension_ime_manager_delegate_impl.h",
     "input_method/emoji_suggester.cc",
     "input_method/emoji_suggester.h",
     "input_method/ime_service_connector.cc",

chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc → chrome/browser/chromeos/input_method/component_extension_ime_manager_delegate_impl.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// Copyright 2020 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 "chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.h"
+#include "chrome/browser/chromeos/input_method/component_extension_ime_manager_delegate_impl.h"
 
 #include <stddef.h>
 
@@ -159,20 +159,24 @@ void OnFilePathChecked(Profile* profile,
 
 }  // namespace
 
-ComponentExtensionIMEManagerImpl::ComponentExtensionIMEManagerImpl() {
+ComponentExtensionIMEManagerDelegateImpl::
+    ComponentExtensionIMEManagerDelegateImpl() {
   ReadComponentExtensionsInfo(&component_extension_list_);
 }
 
-ComponentExtensionIMEManagerImpl::~ComponentExtensionIMEManagerImpl() = default;
+ComponentExtensionIMEManagerDelegateImpl::
+    ~ComponentExtensionIMEManagerDelegateImpl() = default;
 
-std::vector<ComponentExtensionIME> ComponentExtensionIMEManagerImpl::ListIME() {
+std::vector<ComponentExtensionIME>
+ComponentExtensionIMEManagerDelegateImpl::ListIME() {
   return component_extension_list_;
 }
 
-void ComponentExtensionIMEManagerImpl::Load(Profile* profile,
-                                            const std::string& extension_id,
-                                            const std::string& manifest,
-                                            const base::FilePath& file_path) {
+void ComponentExtensionIMEManagerDelegateImpl::Load(
+    Profile* profile,
+    const std::string& extension_id,
+    const std::string& manifest,
+    const base::FilePath& file_path) {
   // Check the existence of file path to avoid unnecessary extension loading
   // and InputMethodEngine creation, so that the virtual keyboard web content
   // url won't be override by IME component extensions.
@@ -189,21 +193,23 @@ void ComponentExtensionIMEManagerImpl::Load(Profile* profile,
 }
 
 std::unique_ptr<base::DictionaryValue>
-ComponentExtensionIMEManagerImpl::GetManifest(
+ComponentExtensionIMEManagerDelegateImpl::GetManifest(
     const std::string& manifest_string) {
   std::string error;
   JSONStringValueDeserializer deserializer(manifest_string);
   std::unique_ptr<base::Value> manifest =
-      deserializer.Deserialize(NULL, &error);
+      deserializer.Deserialize(nullptr, &error);
   if (!manifest.get())
     LOG(ERROR) << "Failed at getting manifest";
 
-  return std::unique_ptr<base::DictionaryValue>(
+  std::unique_ptr<base::DictionaryValue> ret(
       static_cast<base::DictionaryValue*>(manifest.release()));
+  return ret;
 }
 
 // static
-bool ComponentExtensionIMEManagerImpl::IsIMEExtensionID(const std::string& id) {
+bool ComponentExtensionIMEManagerDelegateImpl::IsIMEExtensionID(
+    const std::string& id) {
   for (auto& extension : allowlisted_component_extensions) {
     if (base::LowerCaseEqualsASCII(id, extension.id))
       return true;
@@ -212,7 +218,7 @@ bool ComponentExtensionIMEManagerImpl::IsIMEExtensionID(const std::string& id) {
 }
 
 // static
-bool ComponentExtensionIMEManagerImpl::ReadEngineComponent(
+bool ComponentExtensionIMEManagerDelegateImpl::ReadEngineComponent(
     const ComponentExtensionIME& component_extension,
     const base::DictionaryValue& dict,
     ComponentExtensionEngine* out) {
@@ -230,14 +236,14 @@ bool ComponentExtensionIMEManagerImpl::ReadEngineComponent(
     out->indicator = "";
 
   std::set<std::string> languages;
-  const base::Value* language_value = NULL;
+  const base::Value* language_value = nullptr;
   if (dict.Get(extensions::manifest_keys::kLanguage, &language_value)) {
     if (language_value->is_string()) {
       std::string language_str;
       language_value->GetAsString(&language_str);
       languages.insert(language_str);
     } else if (language_value->is_list()) {
-      const base::ListValue* language_list = NULL;
+      const base::ListValue* language_list = nullptr;
       language_value->GetAsList(&language_list);
       for (size_t j = 0; j < language_list->GetSize(); ++j) {
         std::string language_str;
@@ -249,7 +255,7 @@ bool ComponentExtensionIMEManagerImpl::ReadEngineComponent(
   DCHECK(!languages.empty());
   out->language_codes.assign(languages.begin(), languages.end());
 
-  const base::ListValue* layouts = NULL;
+  const base::ListValue* layouts = nullptr;
   if (!dict.GetList(extensions::manifest_keys::kLayouts, &layouts))
     return false;
 
@@ -270,8 +276,7 @@ bool ComponentExtensionIMEManagerImpl::ReadEngineComponent(
     return false;
   out->input_view_url = url;
 #else
-  if (dict.GetString(extensions::manifest_keys::kInputView,
-                     &url_string)) {
+  if (dict.GetString(extensions::manifest_keys::kInputView, &url_string)) {
     GURL url = extensions::Extension::GetResourceURL(
         extensions::Extension::GetBaseURLFromExtensionId(
             component_extension.id),
@@ -282,8 +287,7 @@ bool ComponentExtensionIMEManagerImpl::ReadEngineComponent(
   }
 #endif
 
-  if (dict.GetString(extensions::manifest_keys::kOptionsPage,
-                     &url_string)) {
+  if (dict.GetString(extensions::manifest_keys::kOptionsPage, &url_string)) {
     GURL url = extensions::Extension::GetResourceURL(
         extensions::Extension::GetBaseURLFromExtensionId(
             component_extension.id),
@@ -300,7 +304,7 @@ bool ComponentExtensionIMEManagerImpl::ReadEngineComponent(
 }
 
 // static
-bool ComponentExtensionIMEManagerImpl::ReadExtensionInfo(
+bool ComponentExtensionIMEManagerDelegateImpl::ReadExtensionInfo(
     const base::DictionaryValue& manifest,
     const std::string& extension_id,
     ComponentExtensionIME* out) {
@@ -325,7 +329,7 @@ bool ComponentExtensionIMEManagerImpl::ReadExtensionInfo(
 }
 
 // static
-void ComponentExtensionIMEManagerImpl::ReadComponentExtensionsInfo(
+void ComponentExtensionIMEManagerDelegateImpl::ReadComponentExtensionsInfo(
     std::vector<ComponentExtensionIME>* out_imes) {
   DCHECK(out_imes);
   for (auto& extension : allowlisted_component_extensions) {

chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.h → chrome/browser/chromeos/input_method/component_extension_ime_manager_delegate_impl.h

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// Copyright 2020 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.
 
-#ifndef CHROME_BROWSER_CHROMEOS_INPUT_METHOD_COMPONENT_EXTENSION_IME_MANAGER_IMPL_H_
-#define CHROME_BROWSER_CHROMEOS_INPUT_METHOD_COMPONENT_EXTENSION_IME_MANAGER_IMPL_H_
+#ifndef CHROME_BROWSER_CHROMEOS_INPUT_METHOD_COMPONENT_EXTENSION_IME_MANAGER_DELEGATE_IMPL_H_
+#define CHROME_BROWSER_CHROMEOS_INPUT_METHOD_COMPONENT_EXTENSION_IME_MANAGER_DELEGATE_IMPL_H_
 
 #include <set>
 #include <vector>
@@ -15,17 +15,18 @@
 #include "base/threading/thread_checker.h"
 #include "base/values.h"
 #include "ui/base/ime/chromeos/component_extension_ime_manager.h"
+#include "ui/base/ime/chromeos/component_extension_ime_manager_delegate.h"
 
 class Profile;
 
 namespace chromeos {
 
 // The implementation class of ComponentExtensionIMEManagerDelegate.
-class ComponentExtensionIMEManagerImpl
+class ComponentExtensionIMEManagerDelegateImpl
     : public ComponentExtensionIMEManagerDelegate {
  public:
-  ComponentExtensionIMEManagerImpl();
-  ~ComponentExtensionIMEManagerImpl() override;
+  ComponentExtensionIMEManagerDelegateImpl();
+  ~ComponentExtensionIMEManagerDelegateImpl() override;
 
   // ComponentExtensionIMEManagerDelegate overrides:
   std::vector<ComponentExtensionIME> ListIME() override;
@@ -63,9 +64,9 @@ class ComponentExtensionIMEManagerImpl
   // The list of component extension IME.
   std::vector<ComponentExtensionIME> component_extension_list_;
 
-  DISALLOW_COPY_AND_ASSIGN(ComponentExtensionIMEManagerImpl);
+  DISALLOW_COPY_AND_ASSIGN(ComponentExtensionIMEManagerDelegateImpl);
 };
 
 }  // namespace chromeos
 
-#endif  // CHROME_BROWSER_CHROMEOS_INPUT_METHOD_COMPONENT_EXTENSION_IME_MANAGER_IMPL_H_
+#endif  // CHROME_BROWSER_CHROMEOS_INPUT_METHOD_COMPONENT_EXTENSION_IME_MANAGER_DELEGATE_IMPL_H_

chrome/browser/chromeos/input_method/input_method_manager_impl.cc

@@ -30,7 +30,7 @@
 #include "chrome/browser/browser_process_platform_part_chromeos.h"
 #include "chrome/browser/chromeos/input_method/assistive_window_controller.h"
 #include "chrome/browser/chromeos/input_method/candidate_window_controller.h"
-#include "chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.h"
+#include "chrome/browser/chromeos/input_method/component_extension_ime_manager_delegate_impl.h"
 #include "chrome/browser/chromeos/input_method/ui/assistive_delegate.h"
 #include "chrome/browser/chromeos/input_method/ui/input_method_menu_item.h"
 #include "chrome/browser/chromeos/input_method/ui/input_method_menu_manager.h"
@@ -46,6 +46,7 @@
 #include "components/user_manager/user_manager.h"
 #include "third_party/icu/source/common/unicode/uloc.h"
 #include "ui/base/ime/chromeos/component_extension_ime_manager.h"
+#include "ui/base/ime/chromeos/component_extension_ime_manager_delegate.h"
 #include "ui/base/ime/chromeos/extension_ime_util.h"
 #include "ui/base/ime/chromeos/fake_ime_keyboard.h"
 #include "ui/base/ime/chromeos/ime_bridge.h"
@@ -946,7 +947,7 @@ InputMethodManagerImpl::InputMethodManagerImpl(
   }
   // Initializes the system IME list.
   std::unique_ptr<ComponentExtensionIMEManagerDelegate> comp_delegate(
-      new ComponentExtensionIMEManagerImpl());
+      new ComponentExtensionIMEManagerDelegateImpl());
   component_extension_ime_manager_->Initialize(std::move(comp_delegate));
   const InputMethodDescriptors& descriptors =
       component_extension_ime_manager_->GetAllIMEAsInputMethodDescriptor();

chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc

@@ -28,6 +28,7 @@
 #include "chrome/test/base/testing_profile_manager.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/ime/chromeos/component_extension_ime_manager.h"
 #include "ui/base/ime/chromeos/extension_ime_util.h"
 #include "ui/base/ime/chromeos/fake_ime_keyboard.h"
 #include "ui/base/ime/chromeos/fake_input_method_delegate.h"

chrome/browser/extensions/component_extensions_allowlist/allowlist.cc

@@ -18,7 +18,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "ash/keyboard/ui/grit/keyboard_resources.h"
-#include "chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.h"
+#include "chrome/browser/chromeos/input_method/component_extension_ime_manager_delegate_impl.h"
 #include "ui/file_manager/grit/file_manager_resources.h"
 #endif
 
@@ -48,7 +48,7 @@ bool IsComponentExtensionAllowlisted(const std::string& extension_id) {
   }
 
 #if defined(OS_CHROMEOS)
-  if (chromeos::ComponentExtensionIMEManagerImpl::IsIMEExtensionID(
+  if (chromeos::ComponentExtensionIMEManagerDelegateImpl::IsIMEExtensionID(
           extension_id)) {
     return true;
   }

ui/base/ime/chromeos/BUILD.gn

@@ -14,6 +14,7 @@ component("chromeos") {
   sources = [
     "component_extension_ime_manager.cc",
     "component_extension_ime_manager.h",
+    "component_extension_ime_manager_delegate.h",
     "extension_ime_util.cc",
     "extension_ime_util.h",
     "fake_ime_keyboard.cc",

ui/base/ime/chromeos/component_extension_ime_manager.cc

@@ -55,12 +55,6 @@ ComponentExtensionIME::ComponentExtensionIME(
 
 ComponentExtensionIME::~ComponentExtensionIME() = default;
 
-ComponentExtensionIMEManagerDelegate::ComponentExtensionIMEManagerDelegate() =
-    default;
-
-ComponentExtensionIMEManagerDelegate::~ComponentExtensionIMEManagerDelegate() =
-    default;
-
 ComponentExtensionIMEManager::ComponentExtensionIMEManager() {
   for (const auto& input_method : input_method::kInputMethods) {
     if (input_method.is_login_keyboard)

ui/base/ime/chromeos/component_extension_ime_manager.h

@@ -13,6 +13,7 @@
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/observer_list.h"
+#include "ui/base/ime/chromeos/component_extension_ime_manager_delegate.h"
 #include "ui/base/ime/chromeos/input_method_descriptor.h"
 
 class Profile;
@@ -47,24 +48,6 @@ struct COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) ComponentExtensionIME {
   std::vector<ComponentExtensionEngine> engines;
 };
 
-// Provides an interface to list/load/unload for component extension IME.
-class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS)
-    ComponentExtensionIMEManagerDelegate {
- public:
-  ComponentExtensionIMEManagerDelegate();
-  virtual ~ComponentExtensionIMEManagerDelegate();
-
-  // Lists installed component extension IMEs.
-  virtual std::vector<ComponentExtensionIME> ListIME() = 0;
-
-  // Loads component extension IME associated with |extension_id|.
-  // Returns false if it fails, otherwise returns true.
-  virtual void Load(Profile* profile,
-                    const std::string& extension_id,
-                    const std::string& manifest,
-                    const base::FilePath& path) = 0;
-};
-
 // This class manages component extension input method.
 class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) ComponentExtensionIMEManager {
  public:

ui/base/ime/chromeos/component_extension_ime_manager_delegate.h

+// Copyright 2020 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.
+
+#ifndef UI_BASE_IME_CHROMEOS_COMPONENT_EXTENSION_IME_MANAGER_DELEGATE_H_
+#define UI_BASE_IME_CHROMEOS_COMPONENT_EXTENSION_IME_MANAGER_DELEGATE_H_
+
+#include "base/component_export.h"
+#include "base/files/file_path.h"
+#include "base/macros.h"
+
+class Profile;
+
+namespace chromeos {
+
+struct ComponentExtensionIME;
+
+// Provides an interface to list/load/unload for component extension IME.
+class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS)
+    ComponentExtensionIMEManagerDelegate {
+ public:
+  virtual ~ComponentExtensionIMEManagerDelegate() {}
+
+  // Lists installed component extension IMEs.
+  virtual std::vector<ComponentExtensionIME> ListIME() = 0;
+
+  // Loads component extension IME associated with |extension_id|.
+  // Returns false if it fails, otherwise returns true.
+  virtual void Load(Profile* profile,
+                    const std::string& extension_id,
+                    const std::string& manifest,
+                    const base::FilePath& path) = 0;
+};
+
+}  // namespace chromeos
+
+#endif  // UI_BASE_IME_CHROMEOS_COMPONENT_EXTENSION_IME_MANAGER_DELEGATE_H_

ui/base/ime/chromeos/mock_component_extension_ime_manager_delegate.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "ui/base/ime/chromeos/mock_component_extension_ime_manager_delegate.h"
+#include "ui/base/ime/chromeos/component_extension_ime_manager.h"
 
 namespace chromeos {
 namespace input_method {

ui/base/ime/chromeos/mock_component_extension_ime_manager_delegate.h

@@ -7,7 +7,7 @@
 
 #include "base/component_export.h"
 #include "base/macros.h"
-#include "ui/base/ime/chromeos/component_extension_ime_manager.h"
+#include "ui/base/ime/chromeos/component_extension_ime_manager_delegate.h"
 
 namespace chromeos {
 namespace input_method {