try 3 of: Split out InputMethodMenuManager from InputMethodManager.

try 3 of: Split out InputMethodMenuManager from InputMethodManager.
(I was missing RemoveObserver in one of the tests and as a result ASAN tests were catching use-after-free.)


Rename input_method_property to input_method_menu_item, and move to ash/ime.


BUG=342336, 343044
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251996

[Junichi Uekawa, 2014-02-19 00:16:46]

Hi,

I was missing a DeleteObserver, caught by ASAN tests. Here's a fixed and updated
change.

[Daniel Erat, 2014-02-19 00:47:45]

still lgtm for system_tray_delegate_chromeos.cc

[Seigo Nonaka, 2014-02-19 03:31:08]

lgtm

[Junichi Uekawa, 2014-02-19 03:35:08]

The CQ bit was checked by uekawa@chromium.org

[commit-bot: I haz the power, 2014-02-19 03:35:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/uekawa@chromium.org/169033003/1

[commit-bot: I haz the power, 2014-02-19 03:35:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-19 03:35:26]

Failed to apply patch for
chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
  Hunk #2 succeeded at 74 (offset 1 line).
  Hunk #3 succeeded at 126 (offset 1 line).
  Hunk #4 succeeded at 159 (offset 1 line).
  Hunk #5 succeeded at 180 (offset 1 line).
  Hunk #6 FAILED at 243.
  Hunk #7 succeeded at 265 (offset 4 lines).
  Hunk #8 succeeded at 643 (offset 42 lines).
  Hunk #9 succeeded at 668 with fuzz 2 (offset 42 lines).
  Hunk #10 succeeded at 694 with fuzz 2 (offset 42 lines).
  1 out of 10 hunks FAILED -- saving rejects to file
chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc.rej

Patch:      
chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
Index:
chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
diff --git
a/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
b/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
index
18428df9d0e26760f21bbd9f360e7a7cc79915ba..ffefb4e08d85db13668b08d472151e6cbe587d54
100644
--- a/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
+++ b/chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc
@@ -6,6 +6,8 @@
 
 #include <algorithm>
 
+#include "ash/ime/input_method_menu_item.h"
+#include "ash/ime/input_method_menu_manager.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -71,6 +73,8 @@ class InputMethodManagerImplTest :  public testing::Test {
     IMEBridge::Initialize();
     IMEBridge::Get()->SetCurrentEngineHandler(mock_engine_handler_.get());
 
+    menu_manager_ = ash::ime::InputMethodMenuManager::GetInstance();
+
     ime_list_.clear();
 
     ComponentExtensionIME ext1;
@@ -121,6 +125,7 @@ class InputMethodManagerImplTest :  public testing::Test {
     candidate_window_controller_ = NULL;
     xkeyboard_ = NULL;
     manager_.reset();
+
     IMEBridge::Get()->SetCurrentEngineHandler(NULL);
     IMEBridge::Shutdown();
   }
@@ -153,16 +158,18 @@ class InputMethodManagerImplTest :  public testing::Test {
   base::MessageLoop message_loop_;
   MockComponentExtIMEManagerDelegate* mock_delegate_;
   std::vector<ComponentExtensionIME> ime_list_;
+  ash::ime::InputMethodMenuManager* menu_manager_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(InputMethodManagerImplTest);
 };
 
-class TestObserver : public InputMethodManager::Observer {
+class TestObserver : public InputMethodManager::Observer,
+                     public ash::ime::InputMethodMenuManager::Observer{
  public:
   TestObserver()
       : input_method_changed_count_(0),
-        input_method_property_changed_count_(0),
+        input_method_menu_item_changed_count_(0),
         last_show_message_(false) {
   }
   virtual ~TestObserver() {}
@@ -172,13 +179,13 @@ class TestObserver : public InputMethodManager::Observer {
     ++input_method_changed_count_;
     last_show_message_ = show_message;
   }
-  virtual void InputMethodPropertyChanged(
-      InputMethodManager* manager) OVERRIDE {
-    ++input_method_property_changed_count_;
+  virtual void InputMethodMenuItemChanged(
+      ash::ime::InputMethodMenuManager* manager) OVERRIDE {
+    ++input_method_menu_item_changed_count_;
   }
 
   int input_method_changed_count_;
-  int input_method_property_changed_count_;
+  int input_method_menu_item_changed_count_;
   bool last_show_message_;
 
  private:
@@ -236,14 +243,15 @@ TEST_F(InputMethodManagerImplTest, TestObserver) {
   TestObserver observer;
   InitComponentExtension();
   manager_->AddObserver(&observer);
+  menu_manager_->AddObserver(&observer);
   EXPECT_EQ(0, observer.input_method_changed_count_);
   manager_->EnableLoginLayouts("en-US", "xkb:us::eng");
   EXPECT_EQ(1, observer.input_method_changed_count_);
-  EXPECT_EQ(1, observer.input_method_property_changed_count_);
+  EXPECT_EQ(1, observer.input_method_menu_item_changed_count_);
   manager_->ChangeInputMethod("xkb:us:dvorak:eng");
   EXPECT_FALSE(observer.last_show_message_);
   EXPECT_EQ(2, observer.input_method_changed_count_);
-  EXPECT_EQ(2, observer.input_method_property_changed_count_);
+  EXPECT_EQ(2, observer.input_method_menu_item_changed_count_);
   manager_->ChangeInputMethod("xkb:us:dvorak:eng");
   EXPECT_FALSE(observer.last_show_message_);
 
@@ -254,9 +262,10 @@ TEST_F(InputMethodManagerImplTest, TestObserver) {
 
   // If the same input method ID is passed, PropertyChanged() is not
   // notified.
-  EXPECT_EQ(2, observer.input_method_property_changed_count_);
+  EXPECT_EQ(2, observer.input_method_menu_item_changed_count_);
 
   manager_->RemoveObserver(&observer);
+  menu_manager_->RemoveObserver(&observer);
 }
 
 TEST_F(InputMethodManagerImplTest, TestGetSupportedInputMethods) {
@@ -593,23 +602,24 @@ TEST_F(InputMethodManagerImplTest, TestXkbSetting) {
   EXPECT_EQ("us(colemak)", xkeyboard_->last_layout_);
 }
 
-TEST_F(InputMethodManagerImplTest, TestActivateInputMethodProperty) {
+TEST_F(InputMethodManagerImplTest, TestActivateInputMethodMenuItem) {
   const std::string kKey = "key";
-  InputMethodPropertyList property_list;
-  property_list.push_back(InputMethodProperty(kKey, "label", false, false));
-  manager_->SetCurrentInputMethodProperties(property_list);
+  ash::ime::InputMethodMenuItemList menu_list;
+  menu_list.push_back(ash::ime::InputMethodMenuItem(
+      kKey, "label", false, false));
+  menu_manager_->SetCurrentInputMethodMenuItemList(menu_list);
 
-  manager_->ActivateInputMethodProperty(kKey);
+  manager_->ActivateInputMethodMenuItem(kKey);
   EXPECT_EQ(kKey, mock_engine_handler_->last_activated_property());
 
   // Key2 is not registered, so activated property should not be changed.
-  manager_->ActivateInputMethodProperty("key2");
+  manager_->ActivateInputMethodMenuItem("key2");
   EXPECT_EQ(kKey, mock_engine_handler_->last_activated_property());
 }
 
 TEST_F(InputMethodManagerImplTest, TestGetCurrentInputMethodProperties) {
   InitComponentExtension();
-  EXPECT_TRUE(manager_->GetCurrentInputMethodProperties().empty());
+  EXPECT_TRUE(menu_manager_->GetCurrentInputMethodMenuItemList().empty());
 
   manager_->SetState(InputMethodManager::STATE_BROWSER_SCREEN);
   std::vector<std::string> ids;
@@ -617,26 +627,25 @@ TEST_F(InputMethodManagerImplTest,
TestGetCurrentInputMethodProperties) {
   ids.push_back(kNaclMozcUsId);
   EXPECT_TRUE(manager_->EnableInputMethods(ids));
   EXPECT_EQ(2U, manager_->GetNumActiveInputMethods());
-  EXPECT_TRUE(manager_->GetCurrentInputMethodProperties().empty());
+  EXPECT_TRUE(menu_manager_->GetCurrentInputMethodMenuItemList().empty());
   manager_->ChangeInputMethod(kNaclMozcUsId);
 
-  InputMethodPropertyList current_property_list;
-  current_property_list.push_back(InputMethodProperty("key",
-                                                      "label",
-                                                      false,
-                                                      false));
-  manager_->SetCurrentInputMethodProperties(current_property_list);
+  ash::ime::InputMethodMenuItemList current_property_list;
+  current_property_list.push_back(ash::ime::InputMethodMenuItem(
+      "key", "label", false, false));
+  menu_manager_->SetCurrentInputMethodMenuItemList(current_property_list);
 
-  ASSERT_EQ(1U, manager_->GetCurrentInputMethodProperties().size());
-  EXPECT_EQ("key", manager_->GetCurrentInputMethodProperties().at(0).key);
+  ASSERT_EQ(1U, menu_manager_->GetCurrentInputMethodMenuItemList().size());
+  EXPECT_EQ("key",
+            menu_manager_->GetCurrentInputMethodMenuItemList().at(0).key);
 
   manager_->ChangeInputMethod("xkb:us::eng");
-  EXPECT_TRUE(manager_->GetCurrentInputMethodProperties().empty());
+  EXPECT_TRUE(menu_manager_->GetCurrentInputMethodMenuItemList().empty());
 }
 
 TEST_F(InputMethodManagerImplTest, TestGetCurrentInputMethodPropertiesTwoImes)
{
   InitComponentExtension();
-  EXPECT_TRUE(manager_->GetCurrentInputMethodProperties().empty());
+  EXPECT_TRUE(menu_manager_->GetCurrentInputMethodMenuItemList().empty());
 
   manager_->SetState(InputMethodManager::STATE_BROWSER_SCREEN);
   std::vector<std::string> ids;
@@ -644,32 +653,31 @@ TEST_F(InputMethodManagerImplTest,
TestGetCurrentInputMethodPropertiesTwoImes) {
   ids.push_back(kExt2Engine1Id);  // T-Chinese
   EXPECT_TRUE(manager_->EnableInputMethods(ids));
   EXPECT_EQ(2U, manager_->GetNumActiveInputMethods());
-  EXPECT_TRUE(manager_->GetCurrentInputMethodProperties().empty());
+  EXPECT_TRUE(menu_manager_->GetCurrentInputMethodMenuItemList().empty());
 
-  InputMethodPropertyList current_property_list;
-  current_property_list.push_back(InputMethodProperty("key-mozc",
-                                                      "label",
-                                                      false,
-                                                      false));
-  manager_->SetCurrentInputMethodProperties(current_property_list);
+  ash::ime::InputMethodMenuItemList current_property_list;
+  current_property_list.push_back(ash::ime::InputMethodMenuItem("key-mozc",
+                                                                "label",
+                                                                false,
+                                                                false));
+  menu_manager_->SetCurrentInputMethodMenuItemList(current_property_list);
 
-  ASSERT_EQ(1U, manager_->GetCurrentInputMethodProperties().size());
-  EXPECT_EQ("key-mozc", manager_->GetCurrentInputMethodProperties().at(0).key);
+  ASSERT_EQ(1U, menu_manager_->GetCurrentInputMethodMenuItemList().size());
+  EXPECT_EQ("key-mozc",
+            menu_manager_->GetCurrentInputMethodMenuItemList().at(0).key);
 
   manager_->ChangeInputMethod(kExt2Engine1Id);
   // Since the IME is changed, the property for mozc Japanese should…
(message too large)

[Junichi Uekawa, 2014-02-19 03:51:49]

The CQ bit was checked by uekawa@chromium.org

[commit-bot: I haz the power, 2014-02-19 03:51:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/uekawa@chromium.org/169033003/380001

[commit-bot: I haz the power, 2014-02-19 08:20:30]

Change committed as 251996