From a5f56931166507a4b9ab3ff635f9af1522bd38e2 Mon Sep 17 00:00:00 2001 From: "shuchen@chromium.org" Date: Tue, 12 Aug 2014 15:27:15 +0000 Subject: [PATCH] Improve perforance for component IME extensions initialization. ComponentExtensionIMEManager::FindEngineEntry() only needs to find for extension. BUG=402222 TEST=Verified on linux_chromeos. Review URL: https://codereview.chromium.org/457943002 Cr-Commit-Position: refs/heads/master@{#288979} git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288979 0039d316-1c4b-4281-b951-d872f2087c98 --- chromeos/ime/component_extension_ime_manager.cc | 132 +++++++-------------- chromeos/ime/component_extension_ime_manager.h | 26 ++-- .../component_extension_ime_manager_unittest.cc | 48 +------- chromeos/ime/extension_ime_util.cc | 18 +-- chromeos/ime/extension_ime_util_unittest.cc | 12 +- 5 files changed, 74 insertions(+), 162 deletions(-) diff --git a/chromeos/ime/component_extension_ime_manager.cc b/chromeos/ime/component_extension_ime_manager.cc index f77421918c29..74a2e09cf4c1 100644 --- a/chromeos/ime/component_extension_ime_manager.cc +++ b/chromeos/ime/component_extension_ime_manager.cc @@ -87,13 +87,27 @@ ComponentExtensionIMEManager::~ComponentExtensionIMEManager() { void ComponentExtensionIMEManager::Initialize( scoped_ptr delegate) { delegate_ = delegate.Pass(); - component_extension_imes_ = delegate_->ListIME(); + std::vector ext_list = delegate_->ListIME(); + for (size_t i = 0; i < ext_list.size(); ++i) { + ComponentExtensionIME& ext = ext_list[i]; + bool extension_exists = IsWhitelistedExtension(ext.id); + if (!extension_exists) + component_extension_imes_[ext.id] = ext; + for (size_t j = 0; j < ext.engines.size(); ++j) { + ComponentExtensionEngine& ime = ext.engines[j]; + const std::string input_method_id = + extension_ime_util::GetComponentInputMethodID(ext.id, ime.engine_id); + if (extension_exists && !IsWhitelisted(input_method_id)) + component_extension_imes_[ext.id].engines.push_back(ime); + input_method_id_set_.insert(input_method_id); + } + } } bool ComponentExtensionIMEManager::LoadComponentExtensionIME( const std::string& input_method_id) { ComponentExtensionIME ime; - if (FindEngineEntry(input_method_id, &ime, NULL)) + if (FindEngineEntry(input_method_id, &ime)) return delegate_->Load(ime.id, ime.manifest, ime.path); else return false; @@ -102,7 +116,7 @@ bool ComponentExtensionIMEManager::LoadComponentExtensionIME( bool ComponentExtensionIMEManager::UnloadComponentExtensionIME( const std::string& input_method_id) { ComponentExtensionIME ime; - if (!FindEngineEntry(input_method_id, &ime, NULL)) + if (!FindEngineEntry(input_method_id, &ime)) return false; delegate_->Unload(ime.id, ime.path); return true; @@ -110,87 +124,41 @@ bool ComponentExtensionIMEManager::UnloadComponentExtensionIME( bool ComponentExtensionIMEManager::IsWhitelisted( const std::string& input_method_id) { - return extension_ime_util::IsComponentExtensionIME(input_method_id) && - FindEngineEntry(input_method_id, NULL, NULL); + return input_method_id_set_.find(input_method_id) != + input_method_id_set_.end(); } bool ComponentExtensionIMEManager::IsWhitelistedExtension( const std::string& extension_id) { - for (size_t i = 0; i < component_extension_imes_.size(); ++i) { - if (component_extension_imes_[i].id == extension_id) - return true; - } - return false; -} - -std::string ComponentExtensionIMEManager::GetId( - const std::string& extension_id, - const std::string& engine_id) { - ComponentExtensionEngine engine; - const std::string& input_method_id = - extension_ime_util::GetComponentInputMethodID(extension_id, engine_id); - if (!FindEngineEntry(input_method_id, NULL, &engine)) - return ""; - return input_method_id; -} - -std::string ComponentExtensionIMEManager::GetName( - const std::string& input_method_id) { - ComponentExtensionEngine engine; - if (!FindEngineEntry(input_method_id, NULL, &engine)) - return ""; - return engine.display_name; -} - -std::string ComponentExtensionIMEManager::GetDescription( - const std::string& input_method_id) { - ComponentExtensionEngine engine; - if (!FindEngineEntry(input_method_id, NULL, &engine)) - return ""; - return engine.description; -} - -std::vector ComponentExtensionIMEManager::ListIMEByLanguage( - const std::string& language) { - std::vector result; - for (size_t i = 0; i < component_extension_imes_.size(); ++i) { - for (size_t j = 0; j < component_extension_imes_[i].engines.size(); ++j) { - const ComponentExtensionIME& ime = component_extension_imes_[i]; - if (std::find(ime.engines[j].language_codes.begin(), - ime.engines[j].language_codes.end(), - language) != ime.engines[j].language_codes.end()) { - result.push_back(extension_ime_util::GetComponentInputMethodID( - ime.id, - ime.engines[j].engine_id)); - } - } - } - return result; + return component_extension_imes_.find(extension_id) != + component_extension_imes_.end(); } input_method::InputMethodDescriptors ComponentExtensionIMEManager::GetAllIMEAsInputMethodDescriptor() { input_method::InputMethodDescriptors result; - for (size_t i = 0; i < component_extension_imes_.size(); ++i) { - for (size_t j = 0; j < component_extension_imes_[i].engines.size(); ++j) { + for (std::map::const_iterator it = + component_extension_imes_.begin(); + it != component_extension_imes_.end(); ++it) { + const ComponentExtensionIME& ext = it->second; + for (size_t j = 0; j < ext.engines.size(); ++j) { + const ComponentExtensionEngine& ime = ext.engines[j]; const std::string input_method_id = extension_ime_util::GetComponentInputMethodID( - component_extension_imes_[i].id, - component_extension_imes_[i].engines[j].engine_id); - const std::vector& layouts = - component_extension_imes_[i].engines[j].layouts; + ext.id, ime.engine_id); + const std::vector& layouts = ime.layouts; result.push_back( input_method::InputMethodDescriptor( input_method_id, - component_extension_imes_[i].engines[j].display_name, + ime.display_name, std::string(), // TODO(uekawa): Set short name. layouts, - component_extension_imes_[i].engines[j].language_codes, + ime.language_codes, // Enables extension based xkb keyboards on login screen. extension_ime_util::IsKeyboardLayoutExtension( input_method_id) && IsInLoginLayoutWhitelist(layouts), - component_extension_imes_[i].engines[j].options_page_url, - component_extension_imes_[i].engines[j].input_view_url)); + ime.options_page_url, + ime.input_view_url)); } } return result; @@ -210,30 +178,20 @@ ComponentExtensionIMEManager::GetXkbIMEAsInputMethodDescriptor() { bool ComponentExtensionIMEManager::FindEngineEntry( const std::string& input_method_id, - ComponentExtensionIME* out_extension, - ComponentExtensionEngine* out_engine) { - if (!extension_ime_util::IsComponentExtensionIME(input_method_id)) + ComponentExtensionIME* out_extension) { + if (!IsWhitelisted(input_method_id)) return false; - for (size_t i = 0; i < component_extension_imes_.size(); ++i) { - const std::string extension_id = component_extension_imes_[i].id; - const std::vector& engines = - component_extension_imes_[i].engines; - for (size_t j = 0; j < engines.size(); ++j) { - const std::string trial_ime_id = - extension_ime_util::GetComponentInputMethodID( - extension_id, engines[j].engine_id); - if (trial_ime_id != input_method_id) - continue; - - if (out_extension) - *out_extension = component_extension_imes_[i]; - if (out_engine) - *out_engine = component_extension_imes_[i].engines[j]; - return true; - } - } - return false; + std::string extension_id = + extension_ime_util::GetExtensionIDFromInputMethodID(input_method_id); + std::map::iterator it = + component_extension_imes_.find(extension_id); + if (it == component_extension_imes_.end()) + return false; + + if (out_extension) + *out_extension = it->second; + return true; } bool ComponentExtensionIMEManager::IsInLoginLayoutWhitelist( diff --git a/chromeos/ime/component_extension_ime_manager.h b/chromeos/ime/component_extension_ime_manager.h index c455712d8f32..ee00fb2ce2a5 100644 --- a/chromeos/ime/component_extension_ime_manager.h +++ b/chromeos/ime/component_extension_ime_manager.h @@ -5,6 +5,7 @@ #ifndef CHROMEOS_IME_COMPONENT_EXTENSION_IME_MANAGER_H_ #define CHROMEOS_IME_COMPONENT_EXTENSION_IME_MANAGER_H_ +#include #include #include "base/files/file_path.h" @@ -88,20 +89,6 @@ class CHROMEOS_EXPORT ComponentExtensionIMEManager { // Returns true if |extension_id| is whitelisted component extension. bool IsWhitelistedExtension(const std::string& extension_id); - // Returns InputMethodId. This function returns empty string if |extension_id| - // and |engine_id| is not a whitelisted component extention IME. - std::string GetId(const std::string& extension_id, - const std::string& engine_id); - - // Returns localized name of |input_method_id|. - std::string GetName(const std::string& input_method_id); - - // Returns localized description of |input_method_id|. - std::string GetDescription(const std::string& input_method_id); - - // Returns list of input method id associated with |language|. - std::vector ListIMEByLanguage(const std::string& language); - // Returns all IME as InputMethodDescriptors. input_method::InputMethodDescriptors GetAllIMEAsInputMethodDescriptor(); @@ -113,14 +100,19 @@ class CHROMEOS_EXPORT ComponentExtensionIMEManager { // |input_method_id|. This function retruns true if it is found, otherwise // returns false. |out_extension| and |out_engine| can be NULL. bool FindEngineEntry(const std::string& input_method_id, - ComponentExtensionIME* out_extension, - ComponentExtensionEngine* out_engine); + ComponentExtensionIME* out_extension); bool IsInLoginLayoutWhitelist(const std::vector& layouts); scoped_ptr delegate_; - std::vector component_extension_imes_; + // The map of extension_id to ComponentExtensionIME instance. + // It's filled by Initialize() method and never changed during runtime. + std::map component_extension_imes_; + + // For quick check the validity of a given input method id. + // It's filled by Initialize() method and never changed during runtime. + std::set input_method_id_set_; std::set login_layout_set_; diff --git a/chromeos/ime/component_extension_ime_manager_unittest.cc b/chromeos/ime/component_extension_ime_manager_unittest.cc index 7822026efb3d..4da6fa32d8d5 100644 --- a/chromeos/ime/component_extension_ime_manager_unittest.cc +++ b/chromeos/ime/component_extension_ime_manager_unittest.cc @@ -21,7 +21,7 @@ class ComponentExtensionIMEManagerTest : public testing::Test { ime_list_.clear(); ComponentExtensionIME ext1; - ext1.id = "ext1_id"; + ext1.id = "ext1_id_xxxxxxxxxxxxxxxxxxxxxxxx"; ext1.description = "ext1_description"; ext1.options_page_url = GURL("chrome-extension://" + ext1.id + "/options.html"); @@ -51,7 +51,7 @@ class ComponentExtensionIMEManagerTest : public testing::Test { ime_list_.push_back(ext1); ComponentExtensionIME ext2; - ext2.id = "ext2_id"; + ext2.id = "ext2_id_xxxxxxxxxxxxxxxxxxxxxxxx"; ext2.description = "ext2_description"; ext2.path = base::FilePath("ext2_file_path"); @@ -79,7 +79,7 @@ class ComponentExtensionIMEManagerTest : public testing::Test { ime_list_.push_back(ext2); ComponentExtensionIME ext3; - ext3.id = "ext3_id"; + ext3.id = "ext3_id_xxxxxxxxxxxxxxxxxxxxxxxx"; ext3.description = "ext3_description"; ext3.options_page_url = GURL("chrome-extension://" + ext3.id + "/options.html"); @@ -181,48 +181,6 @@ TEST_F(ComponentExtensionIMEManagerTest, IsWhitelistedExtensionTest) { EXPECT_FALSE(component_ext_mgr_->IsWhitelistedExtension("")); } -TEST_F(ComponentExtensionIMEManagerTest, GetNameDescriptionTest) { - for (size_t i = 0; i < ime_list_.size(); ++i) { - for (size_t j = 0; j < ime_list_[i].engines.size(); ++j) { - const ComponentExtensionEngine& engine - = ime_list_[i].engines[j]; - - const std::string input_method_id = - extension_ime_util::GetComponentInputMethodID( - ime_list_[i].id, - engine.engine_id); - - EXPECT_EQ(input_method_id, - component_ext_mgr_->GetId(ime_list_[i].id, engine.engine_id)); - EXPECT_EQ(engine.display_name, - component_ext_mgr_->GetName(input_method_id)); - EXPECT_EQ(engine.description, - component_ext_mgr_->GetDescription(input_method_id)); - } - } -} - -TEST_F(ComponentExtensionIMEManagerTest, ListIMEByLanguageTest) { - const std::string hindi_layout1 = - extension_ime_util::GetComponentInputMethodID( - ime_list_[1].id, ime_list_[1].engines[1].engine_id); - const std::string hindi_layout2 = - extension_ime_util::GetComponentInputMethodID( - ime_list_[2].id, ime_list_[2].engines[0].engine_id); - - std::vector hindi_list - = component_ext_mgr_->ListIMEByLanguage("hi"); - ASSERT_EQ(2UL, hindi_list.size()); - EXPECT_TRUE(hindi_list[0] == hindi_layout1 || hindi_list[0] == hindi_layout2); - EXPECT_TRUE(hindi_list[1] == hindi_layout1 || hindi_list[1] == hindi_layout2); - - EXPECT_EQ(0UL, component_ext_mgr_->ListIMEByLanguage("ru").size()); - EXPECT_EQ(0UL, component_ext_mgr_->ListIMEByLanguage("").size()); - EXPECT_EQ(0UL, component_ext_mgr_->ListIMEByLanguage("invalid").size()); - EXPECT_EQ(5UL, component_ext_mgr_->ListIMEByLanguage("en").size()); - EXPECT_EQ(2UL, component_ext_mgr_->ListIMEByLanguage("ja").size()); -} - TEST_F(ComponentExtensionIMEManagerTest, GetAllIMEAsInputMethodDescriptor) { input_method::InputMethodDescriptors descriptors = component_ext_mgr_->GetAllIMEAsInputMethodDescriptor(); diff --git a/chromeos/ime/extension_ime_util.cc b/chromeos/ime/extension_ime_util.cc index 322635f5e478..ee37cae714aa 100644 --- a/chromeos/ime/extension_ime_util.cc +++ b/chromeos/ime/extension_ime_util.cc @@ -39,15 +39,11 @@ std::string GetComponentInputMethodID(const std::string& extension_id, std::string GetExtensionIDFromInputMethodID( const std::string& input_method_id) { - if (IsExtensionIME(input_method_id) && - input_method_id.size() >= kExtensionIMEPrefixLength + - kExtensionIdLength) { + if (IsExtensionIME(input_method_id)) { return input_method_id.substr(kExtensionIMEPrefixLength, kExtensionIdLength); } - if (IsComponentExtensionIME(input_method_id) && - input_method_id.size() >= kComponentExtensionIMEPrefixLength + - kExtensionIdLength) { + if (IsComponentExtensionIME(input_method_id)) { return input_method_id.substr(kComponentExtensionIMEPrefixLength, kExtensionIdLength); } @@ -99,20 +95,24 @@ std::string GetInputMethodIDByEngineID(const std::string& engine_id) { bool IsExtensionIME(const std::string& input_method_id) { return StartsWithASCII(input_method_id, kExtensionIMEPrefix, - true); // Case sensitive. + true /* Case sensitive */) && + input_method_id.size() > kExtensionIMEPrefixLength + + kExtensionIdLength; } bool IsComponentExtensionIME(const std::string& input_method_id) { return StartsWithASCII(input_method_id, kComponentExtensionIMEPrefix, - true); // Case sensitive. + true /* Case sensitive */) && + input_method_id.size() > kComponentExtensionIMEPrefixLength + + kExtensionIdLength; } bool IsMemberOfExtension(const std::string& input_method_id, const std::string& extension_id) { return StartsWithASCII(input_method_id, kExtensionIMEPrefix + extension_id, - true); // Case sensitive. + true /* Case sensitive */); } bool IsKeyboardLayoutExtension(const std::string& input_method_id) { diff --git a/chromeos/ime/extension_ime_util_unittest.cc b/chromeos/ime/extension_ime_util_unittest.cc index e499e8b73c22..bc86f17baeb6 100644 --- a/chromeos/ime/extension_ime_util_unittest.cc +++ b/chromeos/ime/extension_ime_util_unittest.cc @@ -37,18 +37,22 @@ TEST(ExtensionIMEUtilTest, GetExtensionIDFromInputMethodIDTest) { TEST(ExtensionIMEUtilTest, IsExtensionIMETest) { EXPECT_TRUE(extension_ime_util::IsExtensionIME( - extension_ime_util::GetInputMethodID("abcde", "12345"))); + extension_ime_util::GetInputMethodID( + "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345"))); EXPECT_FALSE(extension_ime_util::IsExtensionIME( - extension_ime_util::GetComponentInputMethodID("abcde", "12345"))); + extension_ime_util::GetComponentInputMethodID( + "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345"))); EXPECT_FALSE(extension_ime_util::IsExtensionIME("")); EXPECT_FALSE(extension_ime_util::IsExtensionIME("mozc")); } TEST(ExtensionIMEUtilTest, IsComponentExtensionIMETest) { EXPECT_TRUE(extension_ime_util::IsComponentExtensionIME( - extension_ime_util::GetComponentInputMethodID("abcde", "12345"))); + extension_ime_util::GetComponentInputMethodID( + "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345"))); EXPECT_FALSE(extension_ime_util::IsComponentExtensionIME( - extension_ime_util::GetInputMethodID("abcde", "12345"))); + extension_ime_util::GetInputMethodID( + "abcde_xxxxxxxxxxxxxxxxxxxxxxxxxx", "12345"))); EXPECT_FALSE(extension_ime_util::IsComponentExtensionIME("")); EXPECT_FALSE(extension_ime_util::IsComponentExtensionIME("mozc")); } -- 2.11.4.GIT