Add supports that allow Autofill to be initiated from textarea field

components/autofill/content/renderer/form_autofill_util.cc

Index: components/autofill/content/renderer/form_autofill_util.cc
diff --git a/components/autofill/content/renderer/form_autofill_util.cc b/components/autofill/content/renderer/form_autofill_util.cc
index 0657f78a4f64bdd7ba7056470281269ed797bb4a..97feae6dde416fe9b1363875b767caa792a32ad4 100644
--- a/components/autofill/content/renderer/form_autofill_util.cc
+++ b/components/autofill/content/renderer/form_autofill_util.cc
@@ -438,6 +438,66 @@ base::string16 InferLabelForElement(const WebFormControlElement& element) {
   return InferLabelFromDivTable(element);
 }
 
+WebString GetValue(const WebFormControlElement& element) {

[Ilya Sherman, 2014/02/26 00:12:07]

nit: Please document each new function you add.  For functions like this one, a
very brief documentation string like "// A simple wrapper to get an element's
value." helps the reader know that there's nothing surprising going on.

[ziran.sun, 2014/02/26 18:11:50]

Done.

+  const WebInputElement* input_element = toWebInputElement(&element);
+  if (IsAutofillableInputElement(input_element)) {
+    return input_element->value();
+  } else if (IsTextAreaElement(element)) {
+    const WebTextAreaElement textarea_element =
+        element.toConst<WebTextAreaElement>();
+    return textarea_element.toConst<WebTextAreaElement>().value();
+  } else {
+    DCHECK(IsSelectElement(element));
+    const WebSelectElement select_element =
+        element.toConst<WebSelectElement>();
+    return select_element.value();
+  }
+}

[Ilya Sherman, 2014/02/26 00:12:07]

You can reformat this method to be briefer and clearer by writing it like as I
have below.  There are several types of changes:

(1) No else stmts after "if (...) return ...;"
(2) Removing duplicated lines, like 446-448.
(3) Removing unneeded curly braces after (1) and (2)

WebString GetValue(const WebFormControlElement& element) {
  const WebInputElement* input_element = toWebInputElement(&element);
  if (IsAutofillableInputElement(input_element))
    return input_element->value();

  if (IsTextAreaElement(element))
    return element.toConst<WebTextAreaElement>().value();

  DCHECK(IsSelectElement(element));
  return element.toConst<WebSelectElement>().value();
}

[ziran.sun, 2014/02/26 18:11:50]

Done.

+
+WebString GetSuggestedValue(const WebFormControlElement& element) {
+  const WebInputElement* input_element = toWebInputElement(&element);
+  if (IsAutofillableInputElement(input_element)) {
+    return input_element->suggestedValue();
+  } else {
+    DCHECK(IsTextAreaElement(element));
+    const WebTextAreaElement textarea_element =
+        element.toConst<WebTextAreaElement>();
+    return textarea_element.suggestedValue();
+  }
+}

[Ilya Sherman, 2014/02/26 00:12:07]

Please similarly reformat this method as well as the ones below.

[ziran.sun, 2014/02/26 18:11:50]

Done.

+
+void SetSelectionRange(WebFormControlElement element, int start, int end) {
+  WebInputElement* input_element = toWebInputElement(&element);
+  if (IsAutofillableInputElement(input_element)) {
+    input_element->setSelectionRange(start, end);
+  } else if (IsTextAreaElement(element)) {
+    WebTextAreaElement textarea_element =
+        element.to<WebTextAreaElement>();

[Ilya Sherman, 2014/02/26 00:12:07]

nit: This looks like it fits on a single line.  It might help to get into the
habit of running clang-format over your code -- that should take care of fixing
up a lot of style nits automatically :)

(That said, I would actually combine this with the line below, as
element.to<WebTextAreaElement>().setSelectionRange(start, end);)

[ziran.sun, 2014/02/26 18:11:50]

Done.

+    textarea_element.setSelectionRange(start, end);
+  }
+}
+
+void SetCommonAttributes(const WebFormControlElement& element,
+                         FormFieldData* field) {
+  const WebInputElement* input_element = toWebInputElement(&element);
+  if (IsAutofillableInputElement(input_element) ||
+      IsTextAreaElement(element)) {

[Ilya Sherman, 2014/02/26 00:12:07]

nit: Please write this as an early return, i.e.

if (!IsAutofillableInputElement(input_element) &&
    !IsTextAreaElement(element))
  return;

This way, you're able to reduce the level of indentation for the main body of
the method, which helps a lot with readability.

[ziran.sun, 2014/02/26 18:11:50]

Done.

+    field->is_autofilled = element.isAutofilled();
+    field->is_focusable = element.isFocusable();
+    if (IsAutofillableInputElement(input_element)) {
+      field->should_autocomplete = input_element->autoComplete();
+      field->text_direction = input_element->directionForFormData() ==
+          "rtl" ? base::i18n::RIGHT_TO_LEFT : base::i18n::LEFT_TO_RIGHT;
+    } else {

[Ilya Sherman, 2014/02/26 00:12:07]

nit: DCHECK(IsTextAreaElement(element);

[ziran.sun, 2014/02/26 18:11:50]

Done.

+      const WebTextAreaElement textarea_element =
+          element.toConst<WebTextAreaElement>();
+      field->should_autocomplete = textarea_element.autoComplete();
+      field->text_direction = textarea_element.directionForFormData() ==
+          "rtl" ? base::i18n::RIGHT_TO_LEFT : base::i18n::LEFT_TO_RIGHT;
+    }
+  }
+}
+
 // Fills |option_strings| with the values of the <option> elements present in
 // |select_element|.
 void GetOptionStringsFromElement(const WebSelectElement& select_element,
@@ -539,12 +599,6 @@ void FillFormField(const FormFieldData& data,
     // returns the default maxlength value.
     input_element->setValue(
         data.value.substr(0, input_element->maxLength()), true);
-    if (is_initiating_node) {
-      int length = input_element->value().length();
-      input_element->setSelectionRange(length, length);
-      // Clear the current IME composition (the underline), if there is one.
-      input_element->document().frame()->unmarkText();
-    }
   } else if (IsTextAreaElement(*field)) {
     WebTextAreaElement text_area = field->to<WebTextAreaElement>();
     if (text_area.value() != data.value) {
@@ -561,6 +615,16 @@ void FillFormField(const FormFieldData& data,
     DCHECK(IsCheckableElement(input_element));
     input_element->setChecked(data.is_checked, true);
   }
+
+  if (is_initiating_node &&
+      ((input_element &&

[Ilya Sherman, 2014/02/26 00:12:07]

nit: No need to check whether |input_element| is null, since both IsTextInput()
and IsMonthInput() null-check their input argument.  So, you can simplify lines
620-622 to have the same condition as line 597.

[ziran.sun, 2014/02/26 18:11:50]

Done.

+        (IsTextInput(input_element) ||
+         IsMonthInput(input_element))) ||

[Ilya Sherman, 2014/02/26 00:12:07]

nit: IMO the code is a bit easier to read if this line isn't wrapped.

[ziran.sun, 2014/02/26 18:11:50]

Done.

+       IsTextAreaElement(*field))) {
+    int length = GetValue(*field).length();
+    SetSelectionRange(*field, length, length);
+    field->document().frame()->unmarkText();

[Ilya Sherman, 2014/02/26 00:12:07]

nit: Please preserve the comment "// Clear the current IME composition (the
underline), if there is one." above this line.

[ziran.sun, 2014/02/26 18:11:50]

Done.

+  }

[Ilya Sherman, 2014/02/26 00:12:07]

Thanks for factoring out this shared logic -- it's much clearer now what's
happening! :)

[ziran.sun, 2014/02/26 18:11:50]

Thanks  for helping me with this :).

 }
 
 // Sets the |field|'s "suggested" (non JS visible) value to the value in |data|.
@@ -582,17 +646,21 @@ void PreviewFormField(const FormFieldData& data,
     input_element->setSuggestedValue(
       data.value.substr(0, input_element->maxLength()));
     input_element->setAutofilled(true);
-    if (is_initiating_node) {
-      // Select the part of the text that the user didn't type.
-      input_element->setSelectionRange(
-          input_element->value().length(),
-          input_element->suggestedValue().length());
-    }
   } else if (IsTextAreaElement(*field)) {
     WebTextAreaElement textarea = field->to<WebTextAreaElement>();
     textarea.setSuggestedValue(data.value);
     field->setAutofilled(true);
   }
+
+  if (is_initiating_node &&
+      ((input_element &&
+        IsTextInput(input_element)) ||
+       IsTextAreaElement(*field))) {

[Ilya Sherman, 2014/02/26 00:12:07]

nit: Please format this as:

if (is_initiating_node &&
    (IsTextInput(input_element) || IsTextAreaElement(*field))) {

(So, remove the |input_element| null check, which is redundant with the
IsTextInput() check, and unwrap the IsTextAreaElement() check.)

[ziran.sun, 2014/02/26 18:11:50]

Done.

+    // Select the part of the text that the user didn't type.
+    int start = GetValue(*field).length();
+    int end = GetSuggestedValue(*field).length();
+    SetSelectionRange(*field, start, end);
+  }
 }
 
 std::string RetrievalMethodToString(
@@ -771,18 +839,15 @@ void WebFormControlElementToFormField(const WebFormControlElement& element,
   if (!IsAutofillableElement(element))
     return;
 
+  SetCommonAttributes(element, field);
+
   const WebInputElement* input_element = toWebInputElement(&element);
   if (IsAutofillableInputElement(input_element)) {
     if (IsTextInput(input_element))
       field->max_length = input_element->maxLength();
 
-    field->is_autofilled = input_element->isAutofilled();
-    field->is_focusable = input_element->isFocusable();
-    field->is_checkable = IsCheckableElement(input_element);
-    field->is_checked = input_element->isChecked();
-    field->should_autocomplete = input_element->autoComplete();
-    field->text_direction = input_element->directionForFormData() == "rtl" ?
-        base::i18n::RIGHT_TO_LEFT : base::i18n::LEFT_TO_RIGHT;
+      field->is_checkable = IsCheckableElement(input_element);
+      field->is_checked = input_element->isChecked();
   } else if (IsTextAreaElement(element)) {
     // Nothing more to do in this case.
   } else if (extract_mask & EXTRACT_OPTIONS) {
@@ -798,15 +863,11 @@ void WebFormControlElementToFormField(const WebFormControlElement& element,
     return;
 
   base::string16 value;
-  if (IsAutofillableInputElement(input_element)) {
-    value = input_element->value();
-  } else if (IsTextAreaElement(element)) {
-    value = element.toConst<WebTextAreaElement>().value();
-  } else {
-    DCHECK(IsSelectElement(element));
-    const WebSelectElement select_element = element.toConst<WebSelectElement>();
-    value = select_element.value();
+  value = GetValue(element);

[Ilya Sherman, 2014/02/26 00:12:07]

nit: Please combine these two lines into one, i.e. base::string16 value =
GetValue(element);

[ziran.sun, 2014/02/26 18:11:50]

Done.

 
+  if (IsSelectElement(element)) {
+    const WebSelectElement select_element =
+        element.toConst<WebSelectElement>();

[Ilya Sherman, 2014/02/26 00:12:07]

nit: This line probably doesn't need to wrap.

[ziran.sun, 2014/02/26 18:11:50]

Done.

     // Convert the |select_element| value to text if requested.
     if (extract_mask & EXTRACT_OPTION_TEXT) {
       WebVector<WebElement> list_items = select_element.listItems();
@@ -971,10 +1032,10 @@ bool WebFormElementToFormData(
   return true;
 }
 
-bool FindFormAndFieldForInputElement(const WebInputElement& element,
-                                     FormData* form,
-                                     FormFieldData* field,
-                                     RequirementsMask requirements) {
+bool FindFormAndFieldForFormControlElement(const WebFormControlElement& element,
+                                           FormData* form,
+                                           FormFieldData* field,
+                                           RequirementsMask requirements) {
   if (!IsAutofillableElement(element))
     return false;
 
@@ -992,7 +1053,7 @@ bool FindFormAndFieldForInputElement(const WebInputElement& element,
                                   field);
 }
 
-void FillForm(const FormData& form, const WebInputElement& element) {
+void FillForm(const FormData& form, const WebFormControlElement& element) {
   WebFormElement form_element = element.form();
   if (form_element.isNull())
     return;
@@ -1033,7 +1094,7 @@ void FillFormForAllElements(const FormData& form_data,
                            &FillFormField);
 }
 
-void PreviewForm(const FormData& form, const WebInputElement& element) {
+void PreviewForm(const FormData& form, const WebFormControlElement& element) {
   WebFormElement form_element = element.form();
   if (form_element.isNull())
     return;
@@ -1046,7 +1107,7 @@ void PreviewForm(const FormData& form, const WebInputElement& element) {
                            &PreviewFormField);
 }
 
-bool ClearPreviewedFormWithElement(const WebInputElement& element,
+bool ClearPreviewedFormWithElement(const WebFormControlElement& element,
                                    bool was_autofilled) {
   WebFormElement form_element = element.form();
   if (form_element.isNull())
@@ -1092,8 +1153,8 @@ bool ClearPreviewedFormWithElement(const WebInputElement& element,
       // selection to be lost. We force selection range to restore the text
       // cursor.
       if (is_initiating_node) {
-        int length = input_element->value().length();
-        input_element->setSelectionRange(length, length);
+        int length = GetValue(*input_element).length();
+        SetSelectionRange(*input_element, length, length);
       }

[Ilya Sherman, 2014/02/26 00:12:07]

Please move this code as well as the comment outside of the if/else stmt, as
you've done above.  This has two advantages:

(1) It's harder to have a bug in just one of the two branches, when they are
meant to be identical.
(2) It's easier for a reader to tell that the two branches are meant to be
identical.

[ziran.sun, 2014/02/26 18:11:50]

Done.

     } else if (IsTextAreaElement(control_element)) {
       WebTextAreaElement text_area = control_element.to<WebTextAreaElement>();
@@ -1103,6 +1164,11 @@ bool ClearPreviewedFormWithElement(const WebInputElement& element,
         control_element.setAutofilled(was_autofilled);
       else
         control_element.setAutofilled(false);
+
+      if (is_initiating_node) {
+        int length = GetValue(control_element).length();
+        SetSelectionRange(control_element, length, length);
+      }
     }
   }
 
@@ -1179,7 +1245,7 @@ bool IsWebElementEmpty(const blink::WebElement& element) {
   return true;
 }
 
-gfx::RectF GetScaledBoundingBox(float scale, WebInputElement* element) {
+gfx::RectF GetScaledBoundingBox(float scale, WebFormControlElement* element) {
   gfx::Rect bounding_box(element->boundsInViewportSpace());
   return gfx::RectF(bounding_box.x() * scale,
                     bounding_box.y() * scale,