Apply reviewer's comments

Author: Swanseo0

flutter/shell/platform/tizen/channels/text_input_channel.cc

@@ -7,7 +7,6 @@
 #include "flutter/shell/platform/common/json_method_codec.h"
 #include "flutter/shell/platform/tizen/flutter_tizen_engine.h"
 #include "flutter/shell/platform/tizen/logger.h"
-
 #ifndef WEARABLE_PROFILE
 #include "flutter/shell/platform/tizen/tizen_autofill.h"
 #endif
@@ -27,7 +26,9 @@ constexpr char kMultilineInputType[] = "TextInputType.multiline";
 constexpr char kUpdateEditingStateMethod[] =
     "TextInputClient.updateEditingState";
 constexpr char kPerformActionMethod[] = "TextInputClient.performAction";
+#ifndef WEARABLE_PROFILE
 constexpr char kRequestAutofillMethod[] = "TextInput.requestAutofill";
+#endif
 constexpr char kSetPlatformViewClient[] = "TextInput.setPlatformViewClient";
 constexpr char kTextCapitalization[] = "textCapitalization";
 constexpr char kTextEnableSuggestions[] = "enableSuggestions";
@@ -69,11 +70,12 @@ TextInputChannel::TextInputChannel(
              std::unique_ptr<MethodResult<rapidjson::Document>> result) {
         HandleMethodCall(call, std::move(result));
       });
+
 #ifndef WEARABLE_PROFILE
-  TizenAutofill& instance = TizenAutofill::GetInstance();
-  instance.SetOnPopup(
+  TizenAutofill& autofill = TizenAutofill::GetInstance();
+  autofill.SetOnPopup(
       [this]() { input_method_context_->PopupAutofillItems(); });
-  instance.SetOnCommit([this](std::string value) { OnCommit(value); });
+  autofill.SetOnCommit([this](std::string value) { OnCommit(value); });
 #endif
 }
 
@@ -249,9 +251,9 @@ void TextInputChannel::HandleMethodCall(
       auto hints_iter = autofill_iter->value.FindMember(kHints);
       if (hints_iter != autofill_iter->value.MemberEnd() &&
           hints_iter->value.IsArray()) {
+        autofill_hints_.clear();
         for (auto hint = hints_iter->value.GetArray().Begin();
              hint != hints_iter->value.GetArray().End(); hint++) {
-          autofill_hints_.clear();
           autofill_hints_.push_back(hint->GetString());
         }
       }
@@ -319,12 +321,9 @@ void TextInputChannel::HandleMethodCall(
           cursor_offset);
     }
     SendStateUpdate();
-  } else if (method.compare(kRequestAutofillMethod) == 0) {
 #ifndef WEARABLE_PROFILE
-    TizenAutofill& instance = TizenAutofill::GetInstance();
-    instance.RequestAutofill(autofill_hints_, autofill_id_);
-#else
-    result->NotImplemented();
+  } else if (method.compare(kRequestAutofillMethod) == 0) {
+    TizenAutofill::GetInstance().RequestAutofill(autofill_hints_, autofill_id_);
 #endif
   } else {
     result->NotImplemented();

flutter/shell/platform/tizen/channels/text_input_channel.h

@@ -81,9 +81,10 @@ class TextInputChannel {
   // https://api.flutter.dev/flutter/services/TextCapitalization.html
   std::string text_capitalization_ = "";
 
+  // The active autofill client id.
   std::string autofill_id_;
 
-  // Hints for request autofill. See availabel options:
+  // A list of autofill hint strings. See available options:
   // https://api.flutter.dev/flutter/services/AutofillHints-class.html
   std::vector<std::string> autofill_hints_;
 };

flutter/shell/platform/tizen/nui_autofill_popup.cc

@@ -11,28 +11,34 @@
 #include "flutter/shell/platform/tizen/tizen_autofill.h"
 
 namespace flutter {
-bool NuiAutofillPopup::OnTouch(Dali::Actor actor,
+
+bool NuiAutofillPopup::Touched(Dali::Actor actor,
                                const Dali::TouchEvent& event) {
   const Dali::PointState::Type state = event.GetState(0);
   if (Dali::PointState::DOWN == state) {
-    auto t = actor.GetProperty(Dali::Actor::Property::NAME).Get<std::string>();
-    on_commit_(t);
-    OnHide();
+    std::string text =
+        actor.GetProperty(Dali::Actor::Property::NAME).Get<std::string>();
+    on_commit_(text);
+    Hide();
   }
   return true;
 }
 
-void NuiAutofillPopup::OnHide() {
-  // TODO : There is a phenomenon where white traces remain for a while when
-  // popup disappears.
+void NuiAutofillPopup::Hide() {
+  // TODO(Swanseo0) : There is a phenomenon where white traces remain for a
+  // while when popup disappears.
   popup_.SetDisplayState(Dali::Toolkit::Popup::HIDDEN);
 }
 
-void NuiAutofillPopup::OnHidden() {
+void NuiAutofillPopup::Hidden() {
   popup_.Unparent();
   popup_.Reset();
 }
 
+void NuiAutofillPopup::OutsideTouched() {
+  Hide();
+}
+
 void NuiAutofillPopup::PrepareAutofill() {
   popup_ = Dali::Toolkit::Popup::New();
   popup_.SetProperty(Dali::Actor::Property::NAME, "popup");
@@ -42,14 +48,16 @@ void NuiAutofillPopup::PrepareAutofill() {
                      Dali::AnchorPoint::TOP_LEFT);
   popup_.SetProperty(Dali::Toolkit::Popup::Property::TAIL_VISIBILITY, false);
   popup_.SetBackgroundColor(Dali::Color::WHITE_SMOKE);
-  popup_.OutsideTouchedSignal().Connect(this, &NuiAutofillPopup::OnHide);
-  popup_.HiddenSignal().Connect(this, &NuiAutofillPopup::OnHidden);
+  popup_.OutsideTouchedSignal().Connect(this,
+                                        &NuiAutofillPopup::OutsideTouched);
+  popup_.HiddenSignal().Connect(this, &NuiAutofillPopup::Hidden);
   popup_.SetProperty(Dali::Toolkit::Popup::Property::BACKING_ENABLED, false);
   popup_.SetProperty(Dali::Toolkit::Popup::Property::AUTO_HIDE_DELAY, 2500);
 }
 
 void NuiAutofillPopup::PopupAutofill(Dali::Actor* actor) {
-  const auto& items = TizenAutofill::GetInstance().GetAutofillItems();
+  const std::vector<std::unique_ptr<AutofillItem>>& items =
+      TizenAutofill::GetInstance().GetAutofillItems();
   if (items.size() > 0) {
     PrepareAutofill();
     Dali::Toolkit::TableView content =
@@ -59,14 +67,15 @@ void NuiAutofillPopup::PopupAutofill(Dali::Actor* actor) {
     content.SetProperty(Dali::Actor::Property::PADDING,
                         Dali::Vector4(10, 10, 0, 0));
     for (uint32_t i = 0; i < items.size(); ++i) {
-      auto label = Dali::Toolkit::TextLabel::New(items[i]->label_);
+      Dali::Toolkit::TextLabel label =
+          Dali::Toolkit::TextLabel::New(items[i]->label_);
       label.SetProperty(Dali::Actor::Property::NAME, items[i]->value_);
       label.SetResizePolicy(Dali::ResizePolicy::DIMENSION_DEPENDENCY,
                             Dali::Dimension::HEIGHT);
       label.SetProperty(Dali::Toolkit::TextLabel::Property::TEXT_COLOR,
                         Dali::Color::WHITE_SMOKE);
       label.SetProperty(Dali::Toolkit::TextLabel::Property::POINT_SIZE, 7.0f);
-      label.TouchedSignal().Connect(this, &NuiAutofillPopup::OnTouch);
+      label.TouchedSignal().Connect(this, &NuiAutofillPopup::Touched);
       content.AddChild(label, Dali::Toolkit::TableView::CellPosition(i, 0));
       content.SetFitHeight(i);
     }

flutter/shell/platform/tizen/nui_autofill_popup.h

@@ -16,18 +16,20 @@ using OnRendering = std::function<void()>;
 
 class NuiAutofillPopup : public Dali::ConnectionTracker {
  public:
-  bool OnTouch(Dali::Actor actor, const Dali::TouchEvent& event);
+  void Hide();
 
-  void OnHide();
+  void Hidden();
 
-  void OnHidden();
+  void OutsideTouched();
 
   void PrepareAutofill();
 
   void PopupAutofill(Dali::Actor* actor);
 
   void SetOnCommit(OnCommit callback) { on_commit_ = callback; }
 
+  bool Touched(Dali::Actor actor, const Dali::TouchEvent& event);
+
  private:
   Dali::Toolkit::Popup popup_;
   OnCommit on_commit_;

flutter/shell/platform/tizen/tizen_autofill.cc

@@ -133,7 +133,7 @@ void TizenAutofill::RegisterAutofillItem(std::string view_id,
   autofill_save_item_set_sensitive_data(save_item, item.sensitive_data_);
   autofill_save_item_set_value(save_item, item.value_.c_str());
 
-  char* app_id;
+  char* app_id = nullptr;
   app_get_id(&app_id);
 
   autofill_save_view_info_h save_view_info = nullptr;

flutter/shell/platform/tizen/tizen_autofill.h

@@ -45,7 +45,7 @@ class TizenAutofill {
   void OnCommit(std::string str) { on_commit_(str); }
 
   void OnPopup() { on_popup_(); }
-#
+
   const std::vector<std::unique_ptr<AutofillItem>>& GetAutofillItems() {
     return response_items_;
   }
@@ -57,7 +57,6 @@ class TizenAutofill {
 
   void InitailizeAutofill();
 
-  // TODO : implement convert flutter hint to tizen hint function
   std::optional<autofill_hint_e> ConvertAutofillHint(std::string hint);
 
   autofill_h autofill_;

flutter/shell/platform/tizen/tizen_view_elementary.cc

@@ -291,10 +291,13 @@ void TizenViewElementary::RegisterEventHandlers() {
                                  this);
 
 #ifndef WEARABLE_PROFILE
-  popup_hide_callback_ = [](void* data, Evas* e, Evas_Object* obj,
-                            void* event_info) { elm_ctxpopup_clear(obj); };
+  evas_object_callbacks_[EVAS_CALLBACK_HIDE] =
+      [](void* data, Evas* e, Evas_Object* obj, void* event_info) {
+        elm_ctxpopup_clear(obj);
+      };
   evas_object_event_callback_add(ctxpopup_, EVAS_CALLBACK_HIDE,
-                                 popup_hide_callback_, nullptr);
+                                 evas_object_callbacks_[EVAS_CALLBACK_HIDE],
+                                 nullptr);
 #endif
 }
 
@@ -319,9 +322,10 @@ void TizenViewElementary::UnregisterEventHandlers() {
   evas_object_event_callback_del(container_, EVAS_CALLBACK_KEY_UP,
                                  evas_object_callbacks_[EVAS_CALLBACK_KEY_UP]);
   evas_object_smart_callback_del(container_, "focused", focused_callback_);
+
 #ifndef WEARABLE_PROFILE
   evas_object_event_callback_del(ctxpopup_, EVAS_CALLBACK_HIDE,
-                                 popup_hide_callback_);
+                                 evas_object_callbacks_[EVAS_CALLBACK_HIDE]);
 #endif
 }
 
@@ -385,7 +389,7 @@ void TizenViewElementary::PrepareInputMethod() {
             item.get());
       }
     }
-    // TODO : Change ctxpopup's position to focused input field.
+    // TODO(Swanseo0) : Change ctxpopup's position to focused input field.
     evas_object_move(ctxpopup_, 0, 0);
     evas_object_show(ctxpopup_);
   });

flutter/shell/platform/tizen/tizen_view_elementary.h

@@ -53,7 +53,6 @@ class TizenViewElementary : public TizenView {
   Evas_Object* image_ = nullptr;
 #ifndef WEARABLE_PROFILE
   Evas_Object* ctxpopup_ = nullptr;
-  Evas_Object_Event_Cb popup_hide_callback_ = nullptr;
 #endif
 
   std::unordered_map<Evas_Callback_Type, Evas_Object_Event_Cb>

flutter/shell/platform/tizen/tizen_window_elementary.cc

@@ -335,10 +335,13 @@ void TizenWindowElementary::RegisterEventHandlers() {
                                  this);
 
 #ifndef WEARABLE_PROFILE
-  popup_hide_callback_ = [](void* data, Evas* e, Evas_Object* obj,
-                            void* event_info) { elm_ctxpopup_clear(obj); };
+  evas_object_callbacks_[EVAS_CALLBACK_HIDE] =
+      [](void* data, Evas* e, Evas_Object* obj, void* event_info) {
+        elm_ctxpopup_clear(obj);
+      };
   evas_object_event_callback_add(ctxpopup_, EVAS_CALLBACK_HIDE,
-                                 popup_hide_callback_, nullptr);
+                                 evas_object_callbacks_[EVAS_CALLBACK_HIDE],
+                                 nullptr);
 #endif
 }
 
@@ -364,9 +367,10 @@ void TizenWindowElementary::UnregisterEventHandlers() {
       evas_object_callbacks_[EVAS_CALLBACK_KEY_DOWN]);
   evas_object_event_callback_del(elm_win_, EVAS_CALLBACK_KEY_UP,
                                  evas_object_callbacks_[EVAS_CALLBACK_KEY_UP]);
+
 #ifndef WEARABLE_PROFILE
   evas_object_event_callback_del(ctxpopup_, EVAS_CALLBACK_HIDE,
-                                 popup_hide_callback_);
+                                 evas_object_callbacks_[EVAS_CALLBACK_HIDE]);
 #endif
 }
 
@@ -464,7 +468,7 @@ void TizenWindowElementary::PrepareInputMethod() {
             item.get());
       }
     }
-    // TODO : Change ctxpopup's position to focused input field.
+    // TODO(Swanseo0) : Change ctxpopup's position to focused input field.
     evas_object_move(ctxpopup_, initial_geometry_.left, initial_geometry_.top);
     evas_object_show(ctxpopup_);
   });

flutter/shell/platform/tizen/tizen_window_elementary.h

@@ -74,7 +74,6 @@ class TizenWindowElementary : public TizenWindow {
   Evas_Object* image_ = nullptr;
 #ifndef WEARABLE_PROFILE
   Evas_Object* ctxpopup_ = nullptr;
-  Evas_Object_Event_Cb popup_hide_callback_ = nullptr;
 #endif
 
   Evas_Smart_Cb rotation_changed_callback_ = nullptr;