Use floating labels for preference forms

chrome/browser/android/preferences/autofill/autofill_profile_bridge.cc

Index: chrome/browser/android/preferences/autofill/autofill_profile_bridge.cc
diff --git a/chrome/browser/android/preferences/autofill/autofill_profile_bridge.cc b/chrome/browser/android/preferences/autofill/autofill_profile_bridge.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c42d19cc21946d2bfa1f7874b1314bd4ca9bd5f6
--- /dev/null
+++ b/chrome/browser/android/preferences/autofill/autofill_profile_bridge.cc
@@ -0,0 +1,119 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/android/preferences/autofill/autofill_profile_bridge.h"
+
+#include <jni.h>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_array.h"
+#include "base/android/jni_string.h"
+#include "base/bind.h"
+#include "jni/AutofillProfileBridge_jni.h"
+#include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_ui.h"
+#include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_ui_component.h"
+#include "third_party/libaddressinput/src/cpp/include/libaddressinput/localization.h"
+#include "ui/base/l10n/l10n_util.h"
+
+using base::android::ConvertJavaStringToUTF8;
+using base::android::ToJavaArrayOfStrings;
+using ::i18n::addressinput::AddressUiComponent;
+using ::i18n::addressinput::BuildComponents;
+using ::i18n::addressinput::GetRegionCodes;

[newt (away), 2015/01/27 01:45:07]

If you're only calling a method once, I'd skip using "using".

[Theresa, 2015/01/28 02:05:42]

Done.

+using ::i18n::addressinput::Localization;
+
+namespace autofill{
+namespace android{
+
+static void GetAvailableCountries(JNIEnv* env,
+                                  jclass clazz,
+                                  jobject j_country_list) {
+  std::vector<std::string> regionCodes = GetRegionCodes();
+  Java_AutofillProfileBridge_stringArrayToList(env,
+                                               ToJavaArrayOfStrings(
+                                                   env, regionCodes).obj(),
+                                               j_country_list);
+}
+
+static void GetAddressUiComponents(JNIEnv* env,
+                                   jclass clazz,
+                                   jstring country_code,
+                                   jstring ui_language_tag,

[newt (away), 2015/01/27 01:45:06]

I'd call this j_language_tag (and j_country_code) for consistency

[Theresa, 2015/01/28 02:05:42]

Done.

+                                   jobject j_id_list,
+                                   jobject j_name_list) {
+  std::string best_language_tag;

[newt (away), 2015/01/27 01:45:06]

We never use this value. Should we? / why does it exist?

[Theresa, 2015/01/28 02:05:42]

It's passed in to the ::i18n::addressinput::BuildComponents function as a
required argument -- BuildComponents starts with this assertion:
assert(best_address_language_tag != NULL); 

+  std::string language_code;
+  std::vector<std::string> componentIds;

[newt (away), 2015/01/27 01:45:07]

Use hacker_case not camelCase  (it's easy to forget this when switching between
Java and C++)

[Theresa, 2015/01/28 02:05:42]

Done.

+  std::vector<std::string> componentLabels;
+  Localization localization;
+  localization.SetGetter(l10n_util::GetStringUTF8);
+
+  // Format ui_langage_tag by replacing _'s with -'s. The Java strings coming
+  // from Android are in the format en_US and the C++ equivalent is en-US
+  std::string language_tag = ConvertJavaStringToUTF8(env, ui_language_tag);

[newt (away), 2015/01/27 01:45:06]

language_tag is never used. Did you mean to use it on line 59?

[Theresa, 2015/01/28 02:05:42]

Yep. Done.

+  std::replace(language_tag.begin(), language_tag.end(), '_', '-');
+
+  std::vector<AddressUiComponent> uiComponents = BuildComponents(
+      ConvertJavaStringToUTF8(env, country_code), localization,
+      ConvertJavaStringToUTF8(env, ui_language_tag), &best_language_tag);
+
+  for (auto uiComponent : uiComponents) {
+    // TODO(twellington): debug localization problem:

[newt (away), 2015/01/27 01:45:06]

You can remove this comment. I filed a bug to track this:
https://code.google.com/p/chromium/issues/detail?id=452293

[Theresa, 2015/01/28 02:05:42]

Done.

+    // If the language is changed in the phone's settings menu after Chrome
+    // settings have been launched, when this method gets triggered the strings
+    // returned are still in the original language. E.g. If I open Chrome
+    // settings then switch from English to Spanish, then go to the autofill
+    // address form, the field labels (obtained from this function) are
+    // still in English. If I back out of settings then reopen settings and go
+    // to the autofill address form, the field labels are in Spanish.
+    componentLabels.push_back(uiComponent.name);
+
+    switch (uiComponent.field) {
+      case ::i18n::addressinput::COUNTRY:
+        componentIds.push_back("country");

[newt (away), 2015/01/27 01:45:07]

Is there not a string constant we could use here instead of hardcoding
"country", "admin_area", etc?

See my comment in AutofillProfileEditor about sharing these constants between
C++ and Java using the java_cpp_enum build rule.

[Theresa, 2015/01/28 02:05:42]

Done.

+        break;
+      case ::i18n::addressinput::ADMIN_AREA:
+        componentIds.push_back("admin_area");
+        break;
+      case ::i18n::addressinput::LOCALITY:
+        componentIds.push_back("locality");
+        break;
+      case ::i18n::addressinput::DEPENDENT_LOCALITY:
+        componentIds.push_back("dependent_locality");
+        break;
+      case ::i18n::addressinput::SORTING_CODE:
+        componentIds.push_back("sorting_code");
+        break;
+      case ::i18n::addressinput::POSTAL_CODE:
+        componentIds.push_back("postal_code");
+        break;
+      case ::i18n::addressinput::STREET_ADDRESS:
+        componentIds.push_back("street_address");
+        break;
+      case ::i18n::addressinput::ORGANIZATION:
+        componentIds.push_back("organization");
+        break;
+      case ::i18n::addressinput::RECIPIENT:
+        componentIds.push_back("recipient");
+        break;
+    }

[newt (away), 2015/01/27 01:45:06]

Adding a default case seems like a good idea:

  default:
    NOTREACHED();

[Theresa, 2015/01/28 02:05:42]

Done.

+  }
+
+  Java_AutofillProfileBridge_stringArrayToList(env,
+                                               ToJavaArrayOfStrings(
+                                                   env, componentIds).obj(),
+                                               j_id_list);
+  Java_AutofillProfileBridge_stringArrayToList(env,
+                                               ToJavaArrayOfStrings(
+                                                   env, componentLabels).obj(),
+                                               j_name_list);
+}
+
+// static
+bool RegisterAutofillProfileBridge(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+} // namespace android
+} // namespace autofill