Add a test for toggling logging in PasswordAutofillAgent

chrome/renderer/autofill/password_autofill_agent_browsertest.cc

 // Copyright (c) 2012 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 "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/test/base/chrome_render_view_test.h"
 #include "components/autofill/content/common/autofill_messages.h"
 #include "components/autofill/content/renderer/autofill_agent.h"
 #include "components/autofill/content/renderer/form_autofill_util.h"
@@ skipping 1029 matching lines @@
       username_element_, kAliceUsername, kAlicePassword));
   CheckTextFieldsDOMState(kAliceUsername, true, kAlicePassword, true);
 
   // Try accepting a suggestion with a password different from the one that was
   // initially sent to the renderer.
   EXPECT_TRUE(password_autofill_->AcceptSuggestion(
       username_element_, kBobUsername, kCarolPassword));
   CheckTextFieldsDOMState(kBobUsername, true, kCarolPassword, true);
 }
 
+// Tests that the PasswordAutofillAgent reacts appropriately to messages about
+// changing the logging state.
+// Note: this seems like better suited for a unit test, but for technical
+// complications was done as a browser test, see the discussion at
+// https://codereview.chromium.org/258473005/.
+TEST_F(PasswordAutofillAgentTest, OnChangeLoggingState) {
+  // Up-cast to access OnMessageReceived, which is private in the agent.
+  IPC::Listener* const agent_listener(password_autofill_);
+
+  EXPECT_FALSE(password_autofill_->logging_state_active());
+
+  AutofillMsg_ChangeLoggingState msg_activate(0, true);
+  EXPECT_TRUE(agent_listener->OnMessageReceived(msg_activate));
+  EXPECT_TRUE(password_autofill_->logging_state_active());
+
+  AutofillMsg_ChangeLoggingState msg_deactivate(0, false);
+  EXPECT_TRUE(agent_listener->OnMessageReceived(msg_deactivate));
+  EXPECT_FALSE(password_autofill_->logging_state_active());
+}

[Ilya Sherman, 2014/04/25 18:25:29]

Hmm, could you instead test that RPCs are or aren't sent, depending on the
logging status?  Conceivably, the code could correctly set
|logging_state_active_|, but then incorrectly read from it.  It's generally most
useful to test externally observable behavior, rather than internal class
details.

[vabr (Chromium), 2014/04/28 14:31:09]

Absolutely, that's a very good point.
Done.

+
 }  // namespace autofill