Removed some GetActiveEntry references in autofill

chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc

Index: chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
index 3465f8d0ebc537627c72adbbbacf192f8d0cf95e..365b9ea6395ae030ca9dd7a785cf0208e35594ba 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -576,10 +576,12 @@ base::WeakPtr<AutofillDialogController> AutofillDialogController::Create(
 void AutofillDialogControllerImpl::Show() {
   dialog_shown_timestamp_ = base::Time::Now();
 
-  content::NavigationEntry* entry =
-      web_contents()->GetController().GetActiveEntry();
-  const GURL& active_url = entry ? entry->GetURL() : web_contents()->GetURL();
-  invoked_from_same_origin_ = active_url.GetOrigin() == source_url_.GetOrigin();
+  // Autocomplete is show in response to a message from the renderer. As such,

[Charlie Reis, 2013/11/19 23:10:56]

nit: shown

[jww, 2013/11/19 23:32:02]

Done.

+  // it can only be made in the context of the current document and we must use
+  // the last committed URL for access checks.

[Ilya Sherman, 2013/11/19 22:56:24]

Hmm, I don't really see the value of this comment.  To me, the comment actually
obfuscates the fact that |invoked_from_same_origin_| is a security
consideration, used to warn users when their data is requested from a
cross-origin frame on the page, rather than from the top-level frame or a frame
on the same domain as the top-level frame.

[Charlie Reis, 2013/11/19 23:10:56]

It's apparently non-obvious, because there's a bug there now.  GetActiveEntry
would return the pending entry if there were one, which is incorrect.
Perhaps it's worth including that in the comment as well (first), and then
explaining why LastCommitted is the correct thing to check against?

[Ilya Sherman, 2013/11/19 23:24:37]

Fair point, though I think that has more to do with the semantics of
GetActiveEntry() being unclear (to me, at least).
If I'm understanding you correctly, you're saying that the goal of this comment
is more or less to explain why LastCommitted is more appropriate than
ActiveEntry.  If so, yes, I think that's fair; but then I'd urge Joel to
explicitly call out GetActiveEntry() in the comment.  Otherwise, without the
context of the code that's changing in this CL, it's not obvious (again, to me)
that that's what's being compared to.  (Even with the context of the code that's
changing in this CL, I didn't realize this was a semantic change.  I thought it
was a purely refactoring change to introduce a clearer method name or tighten an
API or something like that.)

[jww, 2013/11/19 23:32:02]

I've uploaded a new comment. @isherman, can you let me know if it is
sufficiently clear or if I can improve it anymore?

+  const GURL& current_url = web_contents()->GetLastCommittedURL();
+  invoked_from_same_origin_ =
+      current_url.GetOrigin() == source_url_.GetOrigin();
 
   // Log any relevant UI metrics and security exceptions.
   GetMetricLogger().LogDialogUiEvent(AutofillMetrics::DIALOG_UI_SHOWN);