Eliminate PasswordAutofillAgentConstructed() in mojo interface PasswordManagerDriver

components/password_manager/content/browser/content_password_manager_driver.cc

 // Copyright 2014 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/password_manager/content/browser/content_password_manager_driver.h"
 
 #include "components/autofill/content/browser/content_autofill_driver.h"
 #include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/content/browser/bad_message.h"
@@ skipping 25 matching lines @@
       client_(client),
       password_generation_manager_(client, this),
       password_autofill_manager_(this, autofill_client),
       next_free_key_(0),
       password_manager_binding_(this),
       weak_factory_(this) {
   // Does nothing if a VisiblePasswordObserver has already been created
   // for this WebContents.
   VisiblePasswordObserver::CreateForWebContents(
       content::WebContents::FromRenderFrameHost(render_frame_host_));
+
+  // For some frames |this| may be instantiated before log manager creation, so
+  // here we can not send logging state to renderer process for them. For such
+  // cases, after the log manager got ready later,
+  // ContentPasswordManagerDriverFactory::RequestSendLoggingAvailability() will
+  // call ContentPasswordManagerDriver::SendLoggingAvailability() on |this| to
+  // do it actually.
+  if (client_->GetLogManager()) {
+    GetPasswordAutofillAgent()->SetLoggingState(

[vabr (Chromium), 2017/02/12 18:48:12]

I acknowledge that calling the SetLoggingState here instead of calling
SendLoggingAvailability is the right thing to do, because the latter is a
virtual function, and those should not be called from a constructor (to avoid
issues with unexpected vtable values).
Optionally you can add a comment to that extent:
// Do not call the virtual method SendLoggingAvailability from a constructor
here, inline its steps instead.

[leonhsl(Using Gerrit), 2017/02/13 00:14:32]

Done.

+        client_->GetLogManager()->IsLoggingActive());
+  }
 }
 
 ContentPasswordManagerDriver::~ContentPasswordManagerDriver() {
 }
 
 // static
 ContentPasswordManagerDriver*
 ContentPasswordManagerDriver::GetForRenderFrameHost(
     content::RenderFrameHost* render_frame_host) {
   ContentPasswordManagerDriverFactory* factory =
@@ skipping 215 matching lines @@
   password_autofill_manager_.OnShowPasswordSuggestions(
       key, text_direction, typed_username, options, bounds);
 }
 
 void ContentPasswordManagerDriver::ShowNotSecureWarning(
     base::i18n::TextDirection text_direction,
     const gfx::RectF& bounds) {
   password_autofill_manager_.OnShowNotSecureWarning(text_direction, bounds);
 }
 
-void ContentPasswordManagerDriver::PasswordAutofillAgentConstructed() {
-  SendLoggingAvailability();
-}
-
 void ContentPasswordManagerDriver::RecordSavePasswordProgress(
     const std::string& log) {
   client_->GetLogManager()->LogSavePasswordProgress(log);
 }
 
 bool ContentPasswordManagerDriver::CheckChildProcessSecurityPolicy(
     const GURL& url,
     BadMessageReason reason) {
   content::ChildProcessSecurityPolicy* policy =
       content::ChildProcessSecurityPolicy::GetInstance();
@@ skipping 34 matching lines @@
 ContentPasswordManagerDriver::GetPasswordGenerationAgent() {
   if (!password_gen_agent_) {
     render_frame_host_->GetRemoteInterfaces()->GetInterface(
         mojo::MakeRequest(&password_gen_agent_));
   }
 
   return password_gen_agent_;
 }
 
 }  // namespace password_manager