Better distinguish didFinishLoad and didStopLoading

components/autofill/content/renderer/password_autofill_agent.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 "components/autofill/content/renderer/password_autofill_agent.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/i18n/case_conversion.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 674 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // PasswordAutofillAgent, public:
 
 PasswordAutofillAgent::PasswordAutofillAgent(content::RenderFrame* render_frame)
     : content::RenderFrameObserver(render_frame),
       legacy_(render_frame->GetRenderView(), this),
       logging_state_active_(false),
       was_username_autofilled_(false),
       was_password_autofilled_(false),
-      did_stop_loading_(false),
       weak_ptr_factory_(this) {
   Send(new AutofillHostMsg_PasswordAutofillAgentConstructed(routing_id()));
 }
 
 PasswordAutofillAgent::~PasswordAutofillAgent() {
 }
 
 PasswordAutofillAgent::PasswordValueGatekeeper::PasswordValueGatekeeper()
     : was_user_gesture_seen_(false) {
 }
@@ skipping 384 matching lines @@
   }
 
   if (password_forms.empty() && !only_visible) {
     // We need to send the PasswordFormsRendered message regardless of whether
     // there are any forms visible, as this is also the code path that triggers
     // showing the infobar.
     return;
   }
 
   if (only_visible) {
-    Send(new AutofillHostMsg_PasswordFormsRendered(routing_id(),
-                                                   password_forms,
-                                                   did_stop_loading_));
+    bool is_last_load = true;
+    for (blink::WebFrame* frame = render_frame()->GetWebFrame()->top(); frame;
+         frame = frame->traverseNext(false)) {
+      if (frame != render_frame()->GetWebFrame() && frame->isLoading()) {

[Nate Chapin, 2015/10/06 18:35:36]

This is kind of gross, and necessary because didStopLoading() is now called
after didFinishLoad(). I guess the alternative is to add this bit as a parameter
in RenderFrameObserver::DidFinishLoad, since RenderFrameImpl can probably
determine it in O(1)?

[dcheng, 2015/10/08 05:34:35]

"didStopLoading does the same thing [no other navigations are in progress], but
fires whether or not the navigation was successful."

With the previous semantics, was it checking for navigations across all frames
in the page, or just navigations in this frame and its descendants?

[Nate Chapin, 2015/10/08 21:03:59]

It was checking all frames. The message blink sends out is per-frame, but
PasswordAutofillAgent was listening to RenderViewObserver::DidStopLoading, which
is per-page.

[dcheng, 2015/10/09 18:34:01]

Hmm... but when this is called, the entire page might not be finished loading
right? So this callback might never run in that case.

[Nate Chapin, 2015/10/09 18:39:44]

This callback is called for each DidFinishLoad. is_last_load should be true if
this will be the last DidFinishLoad in this page (i.e., if this frame is the
only one where isLoading() is true. So it will only not get called if the last
frame fails, which is no change from the status quo.

[dcheng, 2015/10/09 18:45:26]

Hmm. I'm thinking of the OOPI case: subframe renderers will never send the IPC
message in that case, right?

+        is_last_load = false;
+        break;
+      }
+    }
+    Send(new AutofillHostMsg_PasswordFormsRendered(routing_id(), password_forms,
+                                                   is_last_load));
   } else {
     Send(new AutofillHostMsg_PasswordFormsParsed(routing_id(), password_forms));
   }
 }
 
 bool PasswordAutofillAgent::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(PasswordAutofillAgent, message)
     IPC_MESSAGE_HANDLER(AutofillMsg_FillPasswordForm, OnFillPasswordForm)
     IPC_MESSAGE_HANDLER(AutofillMsg_SetLoggingState, OnSetLoggingState)
@@ skipping 25 matching lines @@
   FrameClosing();
 }
 
 void PasswordAutofillAgent::DidCommitProvisionalLoad(
     bool is_new_navigation, bool is_same_page_navigation) {
   if (is_same_page_navigation) {
     OnSamePageNavigationCompleted();
   }
 }
 
-void PasswordAutofillAgent::DidStartLoading() {
-  did_stop_loading_ = false;
-}
-
-void PasswordAutofillAgent::DidStopLoading() {
-  did_stop_loading_ = true;
-}
-
 void PasswordAutofillAgent::FrameDetached() {
   // If a sub frame has been destroyed while the user was entering information
   // into a password form, try to save the data. See https://crbug.com/450806
   // for examples of sites that perform login using this technique.
   if (render_frame()->GetWebFrame()->parent() &&
       ProvisionallySavedPasswordIsValid()) {
     Send(new AutofillHostMsg_InPageNavigation(routing_id(),
                                               *provisionally_saved_form_));
   }
   FrameClosing();
@@ skipping 388 matching lines @@
 }
 
 PasswordAutofillAgent::LegacyPasswordAutofillAgent::
     ~LegacyPasswordAutofillAgent() {
 }
 
 void PasswordAutofillAgent::LegacyPasswordAutofillAgent::OnDestruct() {
   // No op. Do not delete |this|.
 }
 
-void PasswordAutofillAgent::LegacyPasswordAutofillAgent::DidStartLoading() {
-  agent_->DidStartLoading();
-}
-
-void PasswordAutofillAgent::LegacyPasswordAutofillAgent::DidStopLoading() {
-  agent_->DidStopLoading();
-}
-
 void PasswordAutofillAgent::LegacyPasswordAutofillAgent::
     DidStartProvisionalLoad(blink::WebLocalFrame* navigated_frame) {
   agent_->LegacyDidStartProvisionalLoad(navigated_frame);
 }
 
 }  // namespace autofill