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

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);

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_;

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());

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

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 */)

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;
 }
 

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.