Removed some GetActiveEntry references in autofill

chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc

 // Copyright 2013 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/ui/autofill/autofill_dialog_controller_impl.h"
 
 #include <algorithm>
 #include <map>
 #include <string>
 
@@ skipping 558 matching lines @@
     const base::Callback<void(const FormStructure*)>& callback) {
   return AutofillDialogControllerImpl::Create(contents,
                                               form_structure,
                                               source_url,
                                               callback);
 }
 
 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);
 
   GetMetricLogger().LogDialogSecurityMetric(
       AutofillMetrics::SECURITY_METRIC_DIALOG_SHOWN);
 
   // Determine what field types should be included in the dialog.
   // Note that RequestingCreditCardInfo() below relies on parsed field types.
   bool has_types = false;
@@ skipping 2907 matching lines @@
   view_->UpdateButtonStrip();
 }
 
 void AutofillDialogControllerImpl::FetchWalletCookie() {
   net::URLRequestContextGetter* request_context = profile_->GetRequestContext();
   signin_helper_.reset(new wallet::WalletSigninHelper(this, request_context));
   signin_helper_->StartWalletCookieValueFetch();
 }
 
 }  // namespace autofill