Password internals page: notify renderer about logging state on client construction

Currently, ChromePasswordManagerClient informs the PasswordAutofillAgent almost every time about changed availability of the internals page. The only exception is when the client is created. As a result, new tabs opened after a chrome://password-manager-internals tab will only cause browser-side logs.

When the client is created, the agent on the renderer side may not exist yet. So it does not help to send messages to the agent during constructing the client.

Instead, this CL introduces a new "ping" IPC message from the renderer to the browser, which the agent uses to explicitly request logging state update from the client at the time the agent is constructed.

BUG=384269

Review URL: https://codereview.chromium.org/336763002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278738 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 70 insertions, 18 deletions

diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc
index 5258723..48b26be 100644
--- a/chrome/browser/password_manager/chrome_password_manager_client.cc
+++ b/chrome/browser/password_manager/chrome_password_manager_client.cc
@@ -189,13 +189,7 @@ void ChromePasswordManagerClient::OnLogRouterAvailabilityChanged(
     return;
   can_use_log_router_ = router_can_be_used;
 
-  if (!web_contents())
-    return;
-
-  // Also inform the renderer process to start or stop logging.
-  web_contents()->GetRenderViewHost()->Send(new AutofillMsg_ChangeLoggingState(
-      web_contents()->GetRenderViewHost()->GetRoutingID(),
-      can_use_log_router_));
+  NotifyRendererOfLoggingAvailability();
 }
 
 void ChromePasswordManagerClient::LogSavePasswordProgress(
@@ -251,6 +245,8 @@ bool ChromePasswordManagerClient::OnMessageReceived(
                         ShowPasswordEditingPopup)
     IPC_MESSAGE_HANDLER(AutofillHostMsg_HidePasswordGenerationPopup,
                         HidePasswordGenerationPopup)
+    IPC_MESSAGE_HANDLER(AutofillHostMsg_PasswordAutofillAgentConstructed,
+                        NotifyRendererOfLoggingAvailability)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -306,6 +302,15 @@ void ChromePasswordManagerClient::ShowPasswordEditingPopup(
 #endif  // defined(USE_AURA) || defined(OS_MACOSX)
 }
 
+void ChromePasswordManagerClient::NotifyRendererOfLoggingAvailability() {
+  if (!web_contents())
+    return;
+
+  web_contents()->GetRenderViewHost()->Send(new AutofillMsg_SetLoggingState(
+      web_contents()->GetRenderViewHost()->GetRoutingID(),
+      can_use_log_router_));
+}
+
 void ChromePasswordManagerClient::CommitFillPasswordForm(
     autofill::PasswordFormFillData* data) {
   driver_.FillPasswordForm(*data);
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.h b/chrome/browser/password_manager/chrome_password_manager_client.h
index 0d04b6f..b9a3103 100644
--- a/chrome/browser/password_manager/chrome_password_manager_client.h
+++ b/chrome/browser/password_manager/chrome_password_manager_client.h
@@ -109,6 +109,10 @@ class ChromePasswordManagerClient
   void ShowPasswordEditingPopup(
       const gfx::RectF& bounds, const autofill::PasswordForm& form);
 
+  // Sends a message to the renderer with the current value of
+  // |can_use_log_router_|.
+  void NotifyRendererOfLoggingAvailability();
+
   Profile* const profile_;
 
   password_manager::ContentPasswordManagerDriver driver_;
diff --git a/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc b/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
index 87c2239..100a3e6 100644
--- a/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
+++ b/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
@@ -43,8 +43,8 @@ class ChromePasswordManagerClientTest : public ChromeRenderViewHostTestHarness {
  protected:
   ChromePasswordManagerClient* GetClient();
 
-  // If the test IPC sink contains an AutofillMsg_ChangeLoggingState message,
-  // then copies its argument into |activation_flag| and returns true. Otherwise
+  // If the test IPC sink contains an AutofillMsg_SetLoggingState message, then
+  // copies its argument into |activation_flag| and returns true. Otherwise
   // returns false.
   bool WasLoggingActivationMessageSent(bool* activation_flag);
 
@@ -72,13 +72,13 @@ ChromePasswordManagerClient* ChromePasswordManagerClientTest::GetClient() {
 
 bool ChromePasswordManagerClientTest::WasLoggingActivationMessageSent(
     bool* activation_flag) {
-  const uint32 kMsgID = AutofillMsg_ChangeLoggingState::ID;
+  const uint32 kMsgID = AutofillMsg_SetLoggingState::ID;
   const IPC::Message* message =
       process()->sink().GetFirstMessageMatching(kMsgID);
   if (!message)
     return false;
   Tuple1<bool> param;
-  AutofillMsg_ChangeLoggingState::Read(message, &param);
+  AutofillMsg_SetLoggingState::Read(message, &param);
   *activation_flag = param.a;
   process()->sink().ClearMessages();
   return true;
@@ -137,6 +137,35 @@ TEST_F(ChromePasswordManagerClientTest, LogSavePasswordProgressNotifyRenderer) {
   EXPECT_FALSE(logging_active);
 }
 
+TEST_F(ChromePasswordManagerClientTest, AnswerToPingsAboutLoggingState_Active) {
+  service_->RegisterReceiver(&receiver_);
+
+  process()->sink().ClearMessages();
+
+  // Ping the client for logging activity update.
+  AutofillHostMsg_PasswordAutofillAgentConstructed msg(0);
+  static_cast<IPC::Listener*>(GetClient())->OnMessageReceived(msg);
+
+  bool logging_active = false;
+  EXPECT_TRUE(WasLoggingActivationMessageSent(&logging_active));
+  EXPECT_TRUE(logging_active);
+
+  service_->UnregisterReceiver(&receiver_);
+}
+
+TEST_F(ChromePasswordManagerClientTest,
+       AnswerToPingsAboutLoggingState_Inactive) {
+  process()->sink().ClearMessages();
+
+  // Ping the client for logging activity update.
+  AutofillHostMsg_PasswordAutofillAgentConstructed msg(0);
+  static_cast<IPC::Listener*>(GetClient())->OnMessageReceived(msg);
+
+  bool logging_active = true;
+  EXPECT_TRUE(WasLoggingActivationMessageSent(&logging_active));
+  EXPECT_FALSE(logging_active);
+}
+
 TEST_F(ChromePasswordManagerClientTest,
        IsAutomaticPasswordSavingEnabledDefaultBehaviourTest) {
   EXPECT_FALSE(GetClient()->IsAutomaticPasswordSavingEnabled());
diff --git a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
index 9ac90021..6226172 100644
--- a/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
+++ b/chrome/renderer/autofill/password_autofill_agent_browsertest.cc
@@ -1299,7 +1299,7 @@ TEST_F(PasswordAutofillAgentTest, OnChangeLoggingState_NoMessage) {
 // Test that logging can be turned on by a message.
 TEST_F(PasswordAutofillAgentTest, OnChangeLoggingState_Activated) {
   // Turn the logging on.
-  AutofillMsg_ChangeLoggingState msg_activate(0, true);
+  AutofillMsg_SetLoggingState msg_activate(0, true);
   // Up-cast to access OnMessageReceived, which is private in the agent.
   EXPECT_TRUE(static_cast<IPC::Listener*>(password_autofill_)
                   ->OnMessageReceived(msg_activate));
@@ -1314,11 +1314,11 @@ TEST_F(PasswordAutofillAgentTest, OnChangeLoggingState_Activated) {
 // Test that logging can be turned off by a message.
 TEST_F(PasswordAutofillAgentTest, OnChangeLoggingState_Deactivated) {
   // Turn the logging on and then off.
-  AutofillMsg_ChangeLoggingState msg_activate(0, /*active=*/true);
+  AutofillMsg_SetLoggingState msg_activate(0, /*active=*/true);
   // Up-cast to access OnMessageReceived, which is private in the agent.
   EXPECT_TRUE(static_cast<IPC::Listener*>(password_autofill_)
                   ->OnMessageReceived(msg_activate));
-  AutofillMsg_ChangeLoggingState msg_deactivate(0, /*active=*/false);
+  AutofillMsg_SetLoggingState msg_deactivate(0, /*active=*/false);
   EXPECT_TRUE(static_cast<IPC::Listener*>(password_autofill_)
                   ->OnMessageReceived(msg_deactivate));
 
@@ -1329,4 +1329,12 @@ TEST_F(PasswordAutofillAgentTest, OnChangeLoggingState_Deactivated) {
   EXPECT_FALSE(message);
 }
 
+// Test that the agent sends an IPC call to get the current activity state of
+// password saving logging soon after construction.
+TEST_F(PasswordAutofillAgentTest, SendsLoggingStateUpdatePingOnConstruction) {
+  const IPC::Message* message = render_thread_->sink().GetFirstMessageMatching(
+      AutofillHostMsg_PasswordAutofillAgentConstructed::ID);
+  EXPECT_TRUE(message);
+}
+
 }  // namespace autofill
diff --git a/components/autofill/content/common/autofill_messages.h b/components/autofill/content/common/autofill_messages.h
index f9a99fe..4eff7e9 100644
--- a/components/autofill/content/common/autofill_messages.h
+++ b/components/autofill/content/common/autofill_messages.h
@@ -117,7 +117,7 @@ IPC_MESSAGE_ROUTED1(AutofillMsg_FillPasswordForm,
 
 // Notification to start (|active| == true) or stop (|active| == false) logging
 // the decisions made about saving the password.
-IPC_MESSAGE_ROUTED1(AutofillMsg_ChangeLoggingState, bool /* active */)
+IPC_MESSAGE_ROUTED1(AutofillMsg_SetLoggingState, bool /* active */)
 
 // Send the heuristic and server field type predictions to the renderer.
 IPC_MESSAGE_ROUTED1(
@@ -199,6 +199,11 @@ IPC_MESSAGE_ROUTED1(AutofillHostMsg_PasswordFormsParsed,
 IPC_MESSAGE_ROUTED1(AutofillHostMsg_PasswordFormsRendered,
                     std::vector<autofill::PasswordForm> /* forms */)
 
+// A ping to the browser that PasswordAutofillAgent was constructed. As a
+// consequence, the browser sends AutofillMsg_SetLoggingState with the current
+// state of the logging activity.
+IPC_MESSAGE_ROUTED0(AutofillHostMsg_PasswordAutofillAgentConstructed);
+
 // Notification that this password form was submitted by the user.
 IPC_MESSAGE_ROUTED1(AutofillHostMsg_PasswordFormSubmitted,
                     autofill::PasswordForm /* form */)
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index 2bcee92..f989b98 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -233,6 +233,7 @@ PasswordAutofillAgent::PasswordAutofillAgent(content::RenderView* render_view)
       was_password_autofilled_(false),
       username_selection_start_(0),
       weak_ptr_factory_(this) {
+  Send(new AutofillHostMsg_PasswordAutofillAgentConstructed(routing_id()));
 }
 
 PasswordAutofillAgent::~PasswordAutofillAgent() {
@@ -544,7 +545,7 @@ 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_ChangeLoggingState, OnChangeLoggingState)
+    IPC_MESSAGE_HANDLER(AutofillMsg_SetLoggingState, OnSetLoggingState)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -792,7 +793,7 @@ void PasswordAutofillAgent::OnFillPasswordForm(
   }
 }
 
-void PasswordAutofillAgent::OnChangeLoggingState(bool active) {
+void PasswordAutofillAgent::OnSetLoggingState(bool active) {
   logging_state_active_ = active;
 }
 
diff --git a/components/autofill/content/renderer/password_autofill_agent.h b/components/autofill/content/renderer/password_autofill_agent.h
index a1ebefc..fb8fe60 100644
--- a/components/autofill/content/renderer/password_autofill_agent.h
+++ b/components/autofill/content/renderer/password_autofill_agent.h
@@ -136,7 +136,7 @@ class PasswordAutofillAgent : public content::RenderViewObserver {
 
   // RenderView IPC handlers:
   void OnFillPasswordForm(const PasswordFormFillData& form_data);
-  void OnChangeLoggingState(bool active);
+  void OnSetLoggingState(bool active);
 
   // Scans the given frame for password forms and sends them up to the browser.
   // If |only_visible| is true, only forms visible in the layout are sent.