These changes *are* for review :)

chrome/browser/autofill/autofill_ie_toolbar_import_win.cc

Index: chrome/browser/autofill/autofill_ie_toolbar_import_win.cc
===================================================================
--- chrome/browser/autofill/autofill_ie_toolbar_import_win.cc	(revision 84722)
+++ chrome/browser/autofill/autofill_ie_toolbar_import_win.cc	(working copy)
@@ -19,6 +19,7 @@
 #include "chrome/browser/autofill/field_types.h"
 #include "chrome/browser/autofill/form_group.h"
 #include "chrome/browser/autofill/personal_data_manager.h"
+#include "chrome/browser/autofill/phone_number_i18n.h"
 #include "chrome/browser/sync/util/data_encryption.h"
 
 using base::win::RegKey;
@@ -131,6 +132,10 @@
 
   bool has_non_empty_fields = false;
 
+  // Phones need to be rebuild.

[dhollowa, 2011/05/13 18:55:35]

nit: s/rebuild/rebuilt/

[GeorgeY, 2011/05/18 17:41:45]

Done.

+  string16 home, home_city, home_country;
+  string16 fax, fax_city, fax_country;
+
   for (uint32 value_index = 0; value_index < key->ValueCount(); ++value_index) {
     std::wstring value_name;
     if (key->ReadName(value_index, &value_name) != ERROR_SUCCESS)
@@ -144,9 +149,49 @@
       if (it->second == CREDIT_CARD_NUMBER) {
         field_value = DecryptCCNumber(field_value);
       }
-      profile->SetInfo(it->second, field_value);
+      switch (it->second) {

[dhollowa, 2011/05/13 18:55:35]

This is getting pretty hard to read with |SetInfo| at line 174 and then below at
185 and 192.  It is not immediately clear how to make this more readable though.
 Do you have any ideas on how we might split out a helper function or something
to deal with phone specific logic?

[GeorgeY, 2011/05/18 17:41:45]

Simplified it

+        case PHONE_HOME_CITY_CODE:
+          home_city = field_value;
+          break;
+        case PHONE_HOME_COUNTRY_CODE:
+          home_country = field_value;
+          break;
+        case PHONE_HOME_CITY_AND_NUMBER:
+        case PHONE_HOME_NUMBER:
+          home = field_value;
+          break;
+        case PHONE_FAX_CITY_CODE:
+          fax_city = field_value;
+          break;
+        case PHONE_FAX_COUNTRY_CODE:
+          fax_country = field_value;
+          break;
+        case PHONE_FAX_NUMBER:
+        case PHONE_FAX_CITY_AND_NUMBER:
+          fax = field_value;
+          break;
+        default:
+          profile->SetInfo(it->second, field_value);
+          break;
+      }
     }
   }
+  string16 constructed_number;
+  if (!home.empty() && autofill_i18n::ConstructPhoneNumber(
+          home_country, home_city, home, std::string("US"),
+          (home_country.empty() ?
+              autofill_i18n::NATIONAL : autofill_i18n::INTERNATIONAL),
+          &constructed_number)) {
+    profile->SetInfo(PHONE_HOME_WHOLE_NUMBER, constructed_number);
+  }
+  if (!fax.empty() && autofill_i18n::ConstructPhoneNumber(
+          fax_country, fax_city, fax, std::string("US"),
+          (fax_country.empty() ?
+              autofill_i18n::NATIONAL : autofill_i18n::INTERNATIONAL),
+          &constructed_number)) {
+    profile->SetInfo(PHONE_FAX_WHOLE_NUMBER, constructed_number);
+  }
+
   return has_non_empty_fields;
 }
 
@@ -215,15 +260,6 @@
     AutofillProfile profile;
     if (ImportSingleProfile(&profile, &key, reg_to_field)) {
       // Combine phones into whole phone #.
-      string16 phone;
-      phone = profile.GetInfo(PHONE_HOME_COUNTRY_CODE);
-      phone.append(profile.GetInfo(PHONE_HOME_CITY_CODE));
-      phone.append(profile.GetInfo(PHONE_HOME_NUMBER));
-      profile.SetInfo(PHONE_HOME_WHOLE_NUMBER, phone);
-      phone = profile.GetInfo(PHONE_FAX_COUNTRY_CODE);
-      phone.append(profile.GetInfo(PHONE_FAX_CITY_CODE));
-      phone.append(profile.GetInfo(PHONE_FAX_NUMBER));
-      profile.SetInfo(PHONE_FAX_WHOLE_NUMBER, phone);
       profiles->push_back(profile);
     }
   }