Autofill: Refactor some FormFieldData pickling code.

components/autofill/core/common/form_field_data.cc

Index: components/autofill/core/common/form_field_data.cc
diff --git a/components/autofill/core/common/form_field_data.cc b/components/autofill/core/common/form_field_data.cc
index 94c6c7198160bcc15aec8f7d1264495aa2294231..f61b3f5e7150b0a606b47c325579cdaf63d2a997 100644
--- a/components/autofill/core/common/form_field_data.cc
+++ b/components/autofill/core/common/form_field_data.cc
@@ -8,6 +8,8 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 
+namespace autofill {
+
 namespace {
 
 // Increment this anytime pickle format is modified as well as provide
@@ -48,9 +50,34 @@ bool ReadAsInt(PickleIterator* iter, T* target_value) {
   return true;
 }
 
-}  // namespace
+bool DeserializeCommonSection1(PickleIterator* iter,
+                               FormFieldData* field_data) {
+  return (iter->ReadString16(&field_data->label) &&

[Evan Stade, 2015/03/23 19:15:14]

nit: remove outer parens

[Lei Zhang, 2015/03/23 21:00:45]

Done.

+          iter->ReadString16(&field_data->name) &&
+          iter->ReadString16(&field_data->value) &&
+          iter->ReadString(&field_data->form_control_type) &&
+          iter->ReadString(&field_data->autocomplete_attribute) &&
+          iter->ReadSizeT(&field_data->max_length) &&
+          iter->ReadBool(&field_data->is_autofilled) &&
+          iter->ReadBool(&field_data->is_checked) &&
+          iter->ReadBool(&field_data->is_checkable) &&
+          iter->ReadBool(&field_data->is_focusable) &&
+          iter->ReadBool(&field_data->should_autocomplete));
+}
 
-namespace autofill {
+bool DeserializeCommonSection2(PickleIterator* iter,
+                               FormFieldData* field_data) {
+  return (ReadAsInt(iter, &field_data->text_direction) &&

[Evan Stade, 2015/03/23 19:15:14]

ditto

[Lei Zhang, 2015/03/23 21:00:45]

Done.

+          ReadStringVector(iter, &field_data->option_values) &&
+          ReadStringVector(iter, &field_data->option_contents));
+}
+
+bool DeserializeVersion2Specific(PickleIterator* iter,
+                                 FormFieldData* field_data) {
+  return ReadAsInt(iter, &field_data->role);
+}
+
+}  // namespace
 
 FormFieldData::FormFieldData()
     : max_length(0),
@@ -79,7 +106,7 @@ bool FormFieldData::SameFieldAs(const FormFieldData& field) const {
           is_focusable == field.is_focusable &&
           should_autocomplete == field.should_autocomplete &&
           role == field.role && text_direction == field.text_direction);
-          // The option values/contents whith are the list of items in the list
+          // The option values/contents which are the list of items in the list
           // of a drop-down are currently not considered part of the identity of
           // a form element. This is debatable, since one might base heuristics
           // on the types of elements that are available. Alternatively, one
@@ -100,17 +127,15 @@ bool FormFieldData::operator<(const FormFieldData& field) const {
   if (autocomplete_attribute > field.autocomplete_attribute) return false;
   if (max_length < field.max_length) return true;
   if (max_length > field.max_length) return false;
-  // Skip is_checked and is_autofilled as in SameFieldAs.
+  // Skip |is_checked| and |is_autofilled| as in SameFieldAs.
   if (is_checkable < field.is_checkable) return true;
   if (is_checkable > field.is_checkable) return false;
   if (is_focusable < field.is_focusable) return true;
   if (is_focusable > field.is_focusable) return false;
   if (should_autocomplete < field.should_autocomplete) return true;
   if (should_autocomplete > field.should_autocomplete) return false;
-  if (role < field.role)
-    return true;
-  if (role > field.role)
-    return false;
+  if (role < field.role) return true;
+  if (role > field.role) return false;
   if (text_direction < field.text_direction) return true;
   if (text_direction > field.text_direction) return false;
   // See operator== above for why we don't check option_values/contents.
@@ -147,41 +172,17 @@ bool DeserializeFormFieldData(PickleIterator* iter,
 
   switch (version) {
     case 1: {
-      if (!iter->ReadString16(&field_data->label) ||
-          !iter->ReadString16(&field_data->name) ||
-          !iter->ReadString16(&field_data->value) ||
-          !iter->ReadString(&field_data->form_control_type) ||
-          !iter->ReadString(&field_data->autocomplete_attribute) ||
-          !iter->ReadSizeT(&field_data->max_length) ||
-          !iter->ReadBool(&field_data->is_autofilled) ||
-          !iter->ReadBool(&field_data->is_checked) ||
-          !iter->ReadBool(&field_data->is_checkable) ||
-          !iter->ReadBool(&field_data->is_focusable) ||
-          !iter->ReadBool(&field_data->should_autocomplete) ||
-          !ReadAsInt(iter, &field_data->text_direction) ||
-          !ReadStringVector(iter, &field_data->option_values) ||
-          !ReadStringVector(iter, &field_data->option_contents)) {
+      if (!DeserializeCommonSection1(iter, field_data) ||
+          !DeserializeCommonSection2(iter, field_data)) {
         LOG(ERROR) << "Could not deserialize FormFieldData from pickle";
         return false;
       }
       break;
     }
     case 2: {
-      if (!iter->ReadString16(&field_data->label) ||
-          !iter->ReadString16(&field_data->name) ||
-          !iter->ReadString16(&field_data->value) ||
-          !iter->ReadString(&field_data->form_control_type) ||
-          !iter->ReadString(&field_data->autocomplete_attribute) ||
-          !iter->ReadSizeT(&field_data->max_length) ||
-          !iter->ReadBool(&field_data->is_autofilled) ||
-          !iter->ReadBool(&field_data->is_checked) ||
-          !iter->ReadBool(&field_data->is_checkable) ||
-          !iter->ReadBool(&field_data->is_focusable) ||
-          !iter->ReadBool(&field_data->should_autocomplete) ||
-          !ReadAsInt(iter, &field_data->role) ||
-          !ReadAsInt(iter, &field_data->text_direction) ||
-          !ReadStringVector(iter, &field_data->option_values) ||
-          !ReadStringVector(iter, &field_data->option_contents)) {
+      if (!DeserializeCommonSection1(iter, field_data) ||
+          !DeserializeVersion2Specific(iter, field_data) ||
+          !DeserializeCommonSection2(iter, field_data)) {
         LOG(ERROR) << "Could not deserialize FormFieldData from pickle";
         return false;
       }