Pass TextDirection from the renderer for the password generation.

The direction is used if instead of the password generation we trigger the
standard password drop-down.

Bug: 865469
Change-Id: I3da15c0446753e69f84d352c7095c40d571030ce
Reviewed-on: https://chromium-review.googlesource.com/1145300Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577468}

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -722,7 +722,7 @@ void ChromePasswordManagerClient::AutomaticGenerationStatusChanged(
           password_manager_client_bindings_.GetCurrentTargetFrame(),
           ui_data.value().bounds);
       driver->GetPasswordAutofillManager()->MaybeShowPasswordSuggestions(
-          element_bounds_in_screen_space);
+          element_bounds_in_screen_space, ui_data.value().text_direction);
     } else {
       PasswordAccessoryController::FromWebContents(web_contents())
           ->OnAutomaticGenerationStatusChanged(false, base::nullopt, nullptr);
@@ -1053,7 +1053,7 @@ void ChromePasswordManagerClient::ShowPasswordGenerationPopup(
   if (!is_manually_triggered &&
       driver->GetPasswordAutofillManager()
           ->MaybeShowPasswordSuggestionsWithGeneration(
-              element_bounds_in_screen_space))
+              element_bounds_in_screen_space, ui_data.text_direction))
     return;
 
   element_bounds_in_screen_space = GetBoundsInScreenSpace(ui_data.bounds);

components/autofill/content/common/autofill_types.mojom

@@ -206,6 +206,7 @@ struct PasswordGenerationUIData {
   gfx.mojom.RectF bounds;
   int32 max_length;
   mojo_base.mojom.String16 generation_element;
+  mojo_base.mojom.TextDirection text_direction;
   PasswordForm password_form;
 };
 

components/autofill/content/common/autofill_types_struct_traits.cc

@@ -6,7 +6,6 @@
 
 #include "base/i18n/rtl.h"
 #include "mojo/public/cpp/base/string16_mojom_traits.h"
-#include "mojo/public/cpp/base/text_direction_mojom_traits.h"
 #include "mojo/public/cpp/base/time_mojom_traits.h"
 #include "ui/gfx/geometry/mojo/geometry_struct_traits.h"
 #include "url/mojom/origin_mojom_traits.h"
@@ -758,6 +757,7 @@ bool StructTraits<autofill::mojom::PasswordGenerationUIDataDataView,
   out->max_length = data.max_length();
 
   if (!data.ReadGenerationElement(&out->generation_element) ||
+      !data.ReadTextDirection(&out->text_direction) ||
       !data.ReadPasswordForm(&out->password_form))
     return false;
 

components/autofill/content/common/autofill_types_struct_traits.h

@@ -23,6 +23,7 @@
 #include "components/autofill/core/common/password_form_generation_data.h"
 #include "components/autofill/core/common/password_generation_util.h"
 #include "components/autofill/core/common/submission_source.h"
+#include "mojo/public/cpp/base/text_direction_mojom_traits.h"
 #include "mojo/public/cpp/bindings/struct_traits.h"
 #include "ui/gfx/geometry/rect_f.h"
 #include "url/origin.h"
@@ -464,6 +465,11 @@ struct StructTraits<autofill::mojom::PasswordGenerationUIDataDataView,
     return r.generation_element;
   }
 
+  static base::i18n::TextDirection text_direction(
+      const autofill::password_generation::PasswordGenerationUIData& r) {
+    return r.text_direction;
+  }
+
   static const autofill::PasswordForm& password_form(
       const autofill::password_generation::PasswordGenerationUIData& r) {
     return r.password_form;

components/autofill/content/common/autofill_types_struct_traits_unittest.cc

@@ -145,6 +145,15 @@ void CreateTestFormsPredictionsMap(FormsPredictionsMap* predictions) {
       PasswordFormFieldPredictionType::PREDICTION_CURRENT_PASSWORD;
 }
 
+void CreatePasswordGenerationUIData(
+    password_generation::PasswordGenerationUIData* data) {
+  data->bounds = gfx::RectF(1, 1, 200, 100);
+  data->max_length = 20;
+  data->generation_element = base::ASCIIToUTF16("generation_element");
+  data->text_direction = base::i18n::RIGHT_TO_LEFT;
+  CreateTestPasswordForm(&data->password_form);
+}
+
 void CheckEqualPasswordFormFillData(const PasswordFormFillData& expected,
                                     const PasswordFormFillData& actual) {
   EXPECT_EQ(expected.form_renderer_id, actual.form_renderer_id);
@@ -187,6 +196,16 @@ void CheckEqualPasswordFormGenerationData(
             actual.confirmation_field_signature.value());
 }
 
+void CheckEqualPassPasswordGenerationUIData(
+    const password_generation::PasswordGenerationUIData& expected,
+    const password_generation::PasswordGenerationUIData& actual) {
+  EXPECT_EQ(expected.bounds, actual.bounds);
+  EXPECT_EQ(expected.max_length, actual.max_length);
+  EXPECT_EQ(expected.generation_element, actual.generation_element);
+  EXPECT_EQ(expected.text_direction, actual.text_direction);
+  EXPECT_EQ(expected.password_form, actual.password_form);
+}
+
 }  // namespace
 
 class AutofillTypeTraitsTestImpl : public testing::Test,
@@ -300,6 +319,14 @@ void ExpectPasswordFormGenerationData(
   closure.Run();
 }
 
+void ExpectPasswordGenerationUIData(
+    const password_generation::PasswordGenerationUIData& expected,
+    base::OnceClosure closure,
+    const password_generation::PasswordGenerationUIData& passed) {
+  CheckEqualPassPasswordGenerationUIData(expected, passed);
+  std::move(closure).Run();
+}
+
 void ExpectPasswordForm(const PasswordForm& expected,
                         const base::Closure& closure,
                         const PasswordForm& passed) {
@@ -412,6 +439,18 @@ TEST_F(AutofillTypeTraitsTestImpl, PassPasswordFormGenerationData) {
   loop.Run();
 }
 
+TEST_F(AutofillTypeTraitsTestImpl, PassPasswordGenerationUIData) {
+  password_generation::PasswordGenerationUIData input;
+  CreatePasswordGenerationUIData(&input);
+
+  base::RunLoop loop;
+  mojom::TypeTraitsTestPtr proxy = GetTypeTraitsTestProxy();
+  proxy->PassPasswordGenerationUIData(
+      input, base::BindOnce(&ExpectPasswordGenerationUIData, input,
+                            loop.QuitClosure()));
+  loop.Run();
+}
+
 TEST_F(AutofillTypeTraitsTestImpl, PassPasswordForm) {
   PasswordForm input;
   CreateTestPasswordForm(&input);

components/autofill/content/renderer/form_autofill_util.cc

@@ -1420,6 +1420,10 @@ bool IsAutofillableElement(const WebFormControlElement& element) {
          IsSelectElement(element) || IsTextAreaElement(element);
 }
 
+bool IsWebElementVisible(const blink::WebElement& element) {
+  return element.IsFocusable();
+}
+
 const base::string16 GetFormIdentifier(const WebFormElement& form) {
   base::string16 identifier = form.GetName().Utf16();
   CR_DEFINE_STATIC_LOCAL(WebString, kId, ("id"));
@@ -1429,8 +1433,18 @@ const base::string16 GetFormIdentifier(const WebFormElement& form) {
   return identifier;
 }
 
-bool IsWebElementVisible(const blink::WebElement& element) {
-  return element.IsFocusable();
+base::i18n::TextDirection GetTextDirectionForElement(
+    const blink::WebFormControlElement& element) {
+  // Use 'text-align: left|right' if set or 'direction' otherwise.
+  // See https://crbug.com/482339
+  base::i18n::TextDirection direction = element.DirectionForFormData() == "rtl"
+                                            ? base::i18n::RIGHT_TO_LEFT
+                                            : base::i18n::LEFT_TO_RIGHT;
+  if (element.AlignmentForFormData() == "left")
+    direction = base::i18n::LEFT_TO_RIGHT;
+  else if (element.AlignmentForFormData() == "right")
+    direction = base::i18n::RIGHT_TO_LEFT;
+  return direction;
 }
 
 std::vector<blink::WebFormControlElement> ExtractAutofillableElementsFromSet(
@@ -1511,15 +1525,7 @@ void WebFormControlElementToFormField(
     field->is_focusable = IsWebElementVisible(element);
     field->should_autocomplete = element.AutoComplete();
 
-    // Use 'text-align: left|right' if set or 'direction' otherwise.
-    // See crbug.com/482339
-    field->text_direction = element.DirectionForFormData() == "rtl"
-                                ? base::i18n::RIGHT_TO_LEFT
-                                : base::i18n::LEFT_TO_RIGHT;
-    if (element.AlignmentForFormData() == "left")
-      field->text_direction = base::i18n::LEFT_TO_RIGHT;
-    else if (element.AlignmentForFormData() == "right")
-      field->text_direction = base::i18n::RIGHT_TO_LEFT;
+    field->text_direction = GetTextDirectionForElement(element);
     field->is_enabled = element.IsEnabled();
     field->is_readonly = element.IsReadOnly();
     field->is_default = element.GetAttribute("value") == element.Value();

components/autofill/content/renderer/form_autofill_util.h

@@ -123,6 +123,10 @@ bool IsWebElementVisible(const blink::WebElement& element);
 // attribute.
 const base::string16 GetFormIdentifier(const blink::WebFormElement& form);
 
+// Returns text alignment for |element|.
+base::i18n::TextDirection GetTextDirectionForElement(
+    const blink::WebFormControlElement& element);
+
 // Returns all the auto-fillable form control elements in |control_elements|.
 std::vector<blink::WebFormControlElement> ExtractAutofillableElementsFromSet(
     const blink::WebVector<blink::WebFormControlElement>& control_elements);

components/autofill/content/renderer/password_generation_agent.cc

@@ -42,6 +42,7 @@ namespace autofill {
 
 namespace {
 
+using autofill::form_util::GetTextDirectionForElement;
 using Logger = autofill::SavePasswordProgressLogger;
 
 // Returns pairs of |PasswordForm| and corresponding |WebFormElement| for all
@@ -451,6 +452,25 @@ void PasswordGenerationAgent::FoundFormsEligibleForGeneration(
   DetermineGenerationElement();
 }
 
+void PasswordGenerationAgent::UserTriggeredGeneratePassword() {
+  if (SetUpUserTriggeredGeneration()) {
+    LogMessage(Logger::STRING_GENERATION_RENDERER_SHOW_MANUAL_GENERATION_POPUP);
+    autofill::password_generation::PasswordGenerationUIData
+        password_generation_ui_data(
+            render_frame()->GetRenderView()->ElementBoundsInWindow(
+                generation_element_),
+            generation_element_.MaxLength(),
+            generation_element_.NameForAutofill().Utf16(),
+            GetTextDirectionForElement(generation_element_),
+            *generation_form_data_->form);
+    // TODO(crbug.com/845458): remove it. The renderer should never invoke the
+    // prompt directly.
+    GetPasswordManagerClient()->ShowManualPasswordGenerationPopup(
+        password_generation_ui_data);
+    generation_popup_shown_ = true;
+  }
+}
+
 void PasswordGenerationAgent::DetermineGenerationElement() {
   if (generation_form_data_) {
     LogMessage(Logger::STRING_GENERATION_RENDERER_FORM_ALREADY_FOUND);
@@ -655,37 +675,27 @@ bool PasswordGenerationAgent::TextDidChangeInTextField(
 void PasswordGenerationAgent::MaybeOfferAutomaticGeneration() {
   // TODO(crbug.com/852309): Add this check to the generation element class.
   if (!is_manually_triggered_) {
-    ShowGenerationPopup(false /* is_manual_generation */);
-  }
-}
-
-void PasswordGenerationAgent::ShowGenerationPopup(bool is_manual_generation) {
-  if (!render_frame() || generation_element_.IsNull())
-    return;
-  LogMessage(
-      is_manual_generation
-          ? Logger::STRING_GENERATION_RENDERER_SHOW_MANUAL_GENERATION_POPUP
-          : Logger::STRING_GENERATION_RENDERER_AUTOMATIC_GENERATION_AVAILABLE);
-  autofill::password_generation::PasswordGenerationUIData
-      password_generation_ui_data(
-          render_frame()->GetRenderView()->ElementBoundsInWindow(
-              generation_element_),
-          generation_element_.MaxLength(),
-          generation_element_.NameForAutofill().Utf16(),
-          *generation_form_data_->form);
-  generation_popup_shown_ = true;
-  if (is_manual_generation) {
-    GetPasswordManagerClient()->ShowManualPasswordGenerationPopup(
-        password_generation_ui_data);
-  } else {
-    GetPasswordManagerClient()->AutomaticGenerationStatusChanged(
-        true, password_generation_ui_data);
+    AutomaticGenerationStatusChanged(true /* available */);
   }
 }
 
 void PasswordGenerationAgent::AutomaticGenerationStatusChanged(bool available) {
   if (available) {
-    ShowGenerationPopup(false /* is_manual_generation */);
+    if (!render_frame() || generation_element_.IsNull())
+      return;
+    LogMessage(
+        Logger::STRING_GENERATION_RENDERER_AUTOMATIC_GENERATION_AVAILABLE);
+    autofill::password_generation::PasswordGenerationUIData
+        password_generation_ui_data(
+            render_frame()->GetRenderView()->ElementBoundsInWindow(
+                generation_element_),
+            generation_element_.MaxLength(),
+            generation_element_.NameForAutofill().Utf16(),
+            GetTextDirectionForElement(generation_element_),
+            *generation_form_data_->form);
+    generation_popup_shown_ = true;
+    GetPasswordManagerClient()->AutomaticGenerationStatusChanged(
+        true, password_generation_ui_data);
   } else {
     // Hide the generation popup.
     GetPasswordManagerClient()->AutomaticGenerationStatusChanged(false,
@@ -725,11 +735,6 @@ void PasswordGenerationAgent::PasswordNoLongerGenerated() {
     GetPasswordManagerClient()->PasswordNoLongerGenerated(*presaved_form);
 }
 
-void PasswordGenerationAgent::UserTriggeredGeneratePassword() {
-  if (SetUpUserTriggeredGeneration())
-    ShowGenerationPopup(true /* is_manual_generation */);
-}
-
 const mojom::PasswordManagerDriverAssociatedPtr&
 PasswordGenerationAgent::GetPasswordManagerDriver() {
   DCHECK(password_agent_);

components/autofill/content/renderer/password_generation_agent.h

@@ -132,9 +132,6 @@ class PasswordGenerationAgent : public content::RenderFrameObserver,
   // not be offered.
   void MaybeOfferAutomaticGeneration();
 
-  // Shows a generation popup.
-  void ShowGenerationPopup(bool is_manual_generation);
-
   // Signals the browser that it should offer or rescind automatic password
   // generation depending whether the user has just focused a form field
   // suitable for generation or has changed focus from such a field.

components/autofill/core/common/password_generation_util.cc

@@ -27,10 +27,12 @@ PasswordGenerationUIData::PasswordGenerationUIData(
     const gfx::RectF& bounds,
     int max_length,
     const base::string16& generation_element,
+    base::i18n::TextDirection text_direction,
     const autofill::PasswordForm& password_form)
     : bounds(bounds),
       max_length(max_length),
       generation_element(generation_element),
+      text_direction(text_direction),
       password_form(password_form) {}
 
 PasswordGenerationUIData::PasswordGenerationUIData() = default;

components/autofill/core/common/password_generation_util.h

@@ -97,6 +97,7 @@ struct PasswordGenerationUIData {
   PasswordGenerationUIData(const gfx::RectF& bounds,
                            int max_length,
                            const base::string16& generation_element,
+                           base::i18n::TextDirection text_direction,
                            const autofill::PasswordForm& password_form);
   PasswordGenerationUIData();
   ~PasswordGenerationUIData();
@@ -112,6 +113,9 @@ struct PasswordGenerationUIData {
   // Name of the password field to which the generation popup is attached.
   base::string16 generation_element;
 
+  // Direction of the text for |generation_element|.
+  base::i18n::TextDirection text_direction;
+
   // The form associated with the password field.
   autofill::PasswordForm password_form;
 };

components/password_manager/core/browser/password_autofill_manager.cc

@@ -256,21 +256,19 @@ void PasswordAutofillManager::OnShowPasswordSuggestions(
 }
 
 bool PasswordAutofillManager::MaybeShowPasswordSuggestions(
-    const gfx::RectF& bounds) {
+    const gfx::RectF& bounds,
+    base::i18n::TextDirection text_direction) {
   if (login_to_password_info_.empty())
     return false;
-  // TODO(crbug.com/865469): Get text direction from the renderer.
-  OnShowPasswordSuggestions(login_to_password_info_.begin()->first,
-                            base::i18n::IsRTL() ? base::i18n::RIGHT_TO_LEFT
-                                                : base::i18n::LEFT_TO_RIGHT,
-                            base::string16(),
-                            autofill::SHOW_ALL | autofill::IS_PASSWORD_FIELD,
-                            bounds);
+  OnShowPasswordSuggestions(
+      login_to_password_info_.begin()->first, text_direction, base::string16(),
+      autofill::SHOW_ALL | autofill::IS_PASSWORD_FIELD, bounds);
   return true;
 }
 
 bool PasswordAutofillManager::MaybeShowPasswordSuggestionsWithGeneration(
-    const gfx::RectF& bounds) {
+    const gfx::RectF& bounds,
+    base::i18n::TextDirection text_direction) {
   if (login_to_password_info_.empty())
     return false;
   std::vector<autofill::Suggestion> suggestions;
@@ -299,11 +297,8 @@ bool PasswordAutofillManager::MaybeShowPasswordSuggestionsWithGeneration(
         metrics_util::SHOW_ALL_SAVED_PASSWORDS_CONTEXT_PASSWORD);
   }
 
-  autofill_client_->ShowAutofillPopup(
-      bounds,
-      base::i18n::IsRTL() ? base::i18n::RIGHT_TO_LEFT
-                          : base::i18n::LEFT_TO_RIGHT,
-      suggestions, false, weak_ptr_factory_.GetWeakPtr());
+  autofill_client_->ShowAutofillPopup(bounds, text_direction, suggestions,
+                                      false, weak_ptr_factory_.GetWeakPtr());
   return true;
 }
 

components/password_manager/core/browser/password_autofill_manager.h

@@ -69,12 +69,15 @@ class PasswordAutofillManager : public autofill::AutofillPopupDelegate {
   // This is currently used for cases in which the automatic generation
   // option is offered through a different UI surface than the popup
   // (e.g. via the keyboard accessory on Android).
-  bool MaybeShowPasswordSuggestions(const gfx::RectF& bounds);
+  bool MaybeShowPasswordSuggestions(const gfx::RectF& bounds,
+                                    base::i18n::TextDirection text_direction);
 
   // If there are relevant credentials for the current frame, shows them with
   // an additional 'generation' option and returns true. Otherwise, does nothing
   // and returns false.
-  bool MaybeShowPasswordSuggestionsWithGeneration(const gfx::RectF& bounds);
+  bool MaybeShowPasswordSuggestionsWithGeneration(
+      const gfx::RectF& bounds,
+      base::i18n::TextDirection text_direction);
 
   // Called when main frame navigates. Not called for in-page navigations.
   void DidNavigateMainFrame();

components/password_manager/core/browser/password_autofill_manager_unittest.cc

@@ -741,7 +741,7 @@ TEST_F(PasswordAutofillManagerTest,
   gfx::RectF element_bounds;
   EXPECT_FALSE(
       password_autofill_manager_->MaybeShowPasswordSuggestionsWithGeneration(
-          element_bounds));
+          element_bounds, base::i18n::RIGHT_TO_LEFT));
 }
 
 TEST_F(PasswordAutofillManagerTest,
@@ -764,13 +764,13 @@ TEST_F(PasswordAutofillManagerTest,
       l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_GENERATE_PASSWORD);
   EXPECT_CALL(*autofill_client,
               ShowAutofillPopup(
-                  element_bounds, _,
+                  element_bounds, base::i18n::RIGHT_TO_LEFT,
                   SuggestionVectorValuesAre(testing::ElementsAreArray(
                       GetSuggestionList({test_username_, generation_string}))),
                   false, _));
   EXPECT_TRUE(
       password_autofill_manager_->MaybeShowPasswordSuggestionsWithGeneration(
-          element_bounds));
+          element_bounds, base::i18n::RIGHT_TO_LEFT));
 
   // Click "Generate password".
   EXPECT_CALL(*client, GeneratePassword());