[rAc Android] Refuse to show rAc dialog if cc info is not requested

[rAc Android] Refuse to show rAc dialog if cc info is not requested

BUG=362271
TEST=AutofillDialogControllerTest
NOTRY=True

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266391

[aruslan1, 2014-04-24 21:34:47]

PTAL thanks!

[Evan Stade, 2014-04-24 21:56:03]

lgtm

[aruslan1, 2014-04-24 22:04:29]

Thanks, Evan -- PTAL at the "disabled" vs "unsupported" comment.

https://codereview.chromium.org/258543005/diff/1/chrome/android/javatests/src...
File
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
(right):

https://codereview.chromium.org/258543005/diff/1/chrome/android/javatests/src...
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java:448:
setUpAndExpectFailedRequestAutocomplete(
These two tests "pass", but the reported autocomplete error is "disabled"
instead of "unsupported".  I think the corresponding desktop tests "pass" in
similar fashion, so it looks like it's expected, but please chime in if it's
not. Thanks!

[Evan Stade, 2014-04-24 22:06:35]

https://codereview.chromium.org/258543005/diff/1/chrome/android/javatests/src...
File
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
(right):

https://codereview.chromium.org/258543005/diff/1/chrome/android/javatests/src...
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java:448:
setUpAndExpectFailedRequestAutocomplete(
On 2014/04/24 22:04:30, aruslan1 wrote:
> These two tests "pass", but the reported autocomplete error is "disabled"
> instead of "unsupported".  I think the corresponding desktop tests "pass" in
> similar fashion, so it looks like it's expected, but please chime in if it's
> not. Thanks!

Yea, IMO the best way to write this test for greatest resiliency is not making
it depend on the type of failure. Also, Unsupported is not going to exist after
all.

[aruslan1, 2014-04-24 22:08:43]

https://codereview.chromium.org/258543005/diff/1/chrome/android/javatests/src...
File
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
(right):

https://codereview.chromium.org/258543005/diff/1/chrome/android/javatests/src...
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java:448:
setUpAndExpectFailedRequestAutocomplete(
On 2014/04/24 22:06:36, Evan Stade wrote:
> On 2014/04/24 22:04:30, aruslan1 wrote:
> > These two tests "pass", but the reported autocomplete error is "disabled"
> > instead of "unsupported".  I think the corresponding desktop tests "pass" in
> > similar fashion, so it looks like it's expected, but please chime in if it's
> > not. Thanks!
> 
> Yea, IMO the best way to write this test for greatest resiliency is not making
> it depend on the type of failure. Also, Unsupported is not going to exist
after
> all.

Great, then we are golden.

[aruslan1, 2014-04-24 22:54:01]

The CQ bit was checked by aruslan@google.com

[commit-bot: I haz the power, 2014-04-24 22:56:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/258543005/40001

[commit-bot: I haz the power, 2014-04-25 08:38:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-25 08:38:51]

Failed to apply patch for
chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc
  Hunk #2 FAILED at 219.
  1 out of 3 hunks FAILED -- saving rejects to file
chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc.rej

Patch:      
chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc
Index: chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc
diff --git
a/chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc
b/chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc
index
26ad4c519e5c04eecf836cf034a4dfd247046bcf..ba7168a84708260e62d4b3f4ddb15470f84d7d72
100644
--- a/chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc
+++ b/chrome/browser/ui/android/autofill/autofill_dialog_controller_android.cc
@@ -190,10 +190,27 @@ void AutofillDialogControllerAndroid::Show() {
   JNIEnv* env = base::android::AttachCurrentThread();
   dialog_shown_timestamp_ = base::Time::Now();
 
+  // The Autofill dialog is shown in response to a message from the renderer
and
+  // as such, it can only be made in the context of the current document. A
call
+  // to GetActiveEntry would return a pending entry, if there was one, which
+  // would be a security bug. Therefore, we use the last committed URL for the
+  // access checks.
   const GURL& current_url = contents_->GetLastCommittedURL();
   invoked_from_same_origin_ =
       current_url.GetOrigin() == source_url_.GetOrigin();
 
+  // Fail if the dialog factory (e.g. SDK) doesn't support cross-origin calls.
+  if (!Java_AutofillDialogControllerAndroid_isDialogAllowed(
+          env,
+          invoked_from_same_origin_)) {
+    callback_.Run(
+        AutofillManagerDelegate::AutocompleteResultErrorDisabled,
+        base::ASCIIToUTF16("Cross-origin form invocations are not supported."),
+        NULL);
+    delete this;
+    return;
+  }
+
   // Determine what field types should be included in the dialog.
   bool has_types = false;
   bool has_sections = false;
@@ -202,18 +219,36 @@ void AutofillDialogControllerAndroid::Show() {
 
   // Fail if the author didn't specify autocomplete types, or
   // if the dialog shouldn't be shown in a given circumstances.
-  if (!has_types ||
-      !Java_AutofillDialogControllerAndroid_isDialogAllowed(
-          env,
-          invoked_from_same_origin_)) {
+  if (!has_types) {
     callback_.Run(
-        AutofillManagerDelegate::AutocompleteResultErrorUnsupported,
+        AutofillManagerDelegate::AutocompleteResultErrorDisabled,
         base::ASCIIToUTF16("Form is missing autocomplete attributes."),
         NULL);
     delete this;
     return;
   }
 
+  // Fail if the author didn't ask for at least some kind of credit card
+  // information.
+  bool has_credit_card_field = false;
+  for (size_t i = 0; i < form_structure_.field_count(); ++i) {
+    AutofillType type = form_structure_.field(i)->Type();
+    if (type.html_type() != HTML_TYPE_UNKNOWN && type.group() == CREDIT_CARD) {
+      has_credit_card_field = true;
+      break;
+    }
+  }
+
+  if (!has_credit_card_field) {
+    callback_.Run(
+        AutofillManagerDelegate::AutocompleteResultErrorDisabled,
+        base::ASCIIToUTF16("Form is not a payment form (must contain "
+                           "some autocomplete=\"cc-*\" fields). "),
+        NULL);
+    delete this;
+    return;
+  }
+
   // Log any relevant UI metrics and security exceptions.
   GetMetricLogger().LogDialogUiEvent(AutofillMetrics::DIALOG_UI_SHOWN);
 
@@ -378,7 +413,7 @@ void AutofillDialogControllerAndroid::DialogContinue(
       if (!last_used_card.empty())
         defaults->SetString(kLastUsedCreditCardGuid, last_used_card);
     } else {
-      LOG(ERROR) << "Failed to save AutofillDialog preferences";
+      DLOG(ERROR) << "Failed to save AutofillDialog preferences";
     }
   }

[aruslan1, 2014-04-25 22:16:52]

The CQ bit was checked by aruslan@google.com

[commit-bot: I haz the power, 2014-04-25 22:26:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/258543005/80001

[commit-bot: I haz the power, 2014-04-26 00:30:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-26 00:30:37]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[aruslan, 2014-04-26 08:15:10]

The CQ bit was checked by aruslan@chromium.org

[commit-bot: I haz the power, 2014-04-26 08:15:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aruslan@chromium.org/258543005/80001

[commit-bot: I haz the power, 2014-04-26 22:35:40]

Change committed as 266391