[Autofill] OnLoadedServerPredictions correctly supplies FormStructure to ParseQueryResponse.

components/autofill/core/browser/autofill_manager.cc

Index: components/autofill/core/browser/autofill_manager.cc
diff --git a/components/autofill/core/browser/autofill_manager.cc b/components/autofill/core/browser/autofill_manager.cc
index 962dffd1b951aebba3c0f90a9fca1b8c96cdfba5..0aca9ee0789382cff5f57f8f3bd989a0ae1fe17d 100644
--- a/components/autofill/core/browser/autofill_manager.cc
+++ b/components/autofill/core/browser/autofill_manager.cc
@@ -6,6 +6,7 @@
 
 #include <stddef.h>
 
+#include <algorithm>
 #include <limits>
 #include <map>
 #include <set>
@@ -745,16 +746,21 @@ void AutofillManager::OnLoadedServerPredictions(
     const std::vector<std::string>& form_signatures) {
   // We obtain the current valid FormStructures represented by
   // |form_signatures|. We invert both lists because most recent forms are at

[vabr (Chromium), 2015/11/23 16:59:25]

Just curious -- was this meant to improve performance? Is it still worth it with
the std::reverse at the end, and the added complexity?

[Mathieu, 2015/11/23 17:27:07]

Yes it's meant to improve performance. With <form>-less Autofill a new form
structure is created at every field added. This pool of forms can then get quite
large. I think complexity is not that bad and replicated elsewhere in this file.

[vabr (Chromium), 2015/11/23 17:47:07]

Acknowledged.

-  // the end of the list.
+  // the end of the list (and reverse the resulting pointer vector).
   std::vector<FormStructure*> queried_forms;
   for (const std::string& signature : base::Reversed(form_signatures)) {
+    bool found_signature = false;
     for (FormStructure* cur_form : base::Reversed(form_structures_)) {
+      if (found_signature)
+        break;
       if (cur_form->FormSignature() == signature) {
         queried_forms.push_back(cur_form);
-        continue;
+        found_signature = true;

[vabr (Chromium), 2015/11/23 16:59:25]

Could you just execute "break;" here and remove |found_signature|? Or did you
mean to use |form_signature| after the for loop to test for success?

[Mathieu, 2015/11/23 17:27:07]

Of course, I did this too quickly, brain fart.

       }
     }
   }
+  std::reverse(queried_forms.begin(), queried_forms.end());
+
   // If there are no current forms corresponding to the queried signatures, drop
   // the query response.
   if (queried_forms.empty())