Password manager internals page should flush on reload

More precisely, if there is a single instance of chrome://password-manager-internals, then it should clear all logs on reload, because during the reload, there is no active instance of the internals page.
When there are multiple such pages, and one reloads, the logs should be preserved, because of the non-reloading pages.

This CL also corrects the user instructions on the internals page itself, to take into account the possibility to open multiple internals pages.

BUG=347927

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

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

5 files changed, 71 insertions, 17 deletions

diff --git a/chrome/browser/resources/password_manager_internals/password_manager_internals.html b/chrome/browser/resources/password_manager_internals/password_manager_internals.html
index b494339..872aab8 100644
--- a/chrome/browser/resources/password_manager_internals/password_manager_internals.html
+++ b/chrome/browser/resources/password_manager_internals/password_manager_internals.html
@@ -12,8 +12,9 @@
 <h1>Password Manager Internals</h1>
 <div id="logging-note">
   <i>
-    Showing login entries since opening this tab.
-    Close the tab to stop logging and to permanently delete existing entries.
+    Showing password manager logs since opening a password-manager-internals
+    page. Close all password-manager-internals pages to stop logging and to
+    permanently delete existing entries.
   </i>
 </div>
 <div id="log-entries">
diff --git a/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui.cc b/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui.cc
index 9096347..28499de 100644
--- a/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui.cc
+++ b/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui.cc
@@ -46,13 +46,12 @@ PasswordManagerInternalsUI::PasswordManagerInternalsUI(content::WebUI* web_ui)
 }
 
 PasswordManagerInternalsUI::~PasswordManagerInternalsUI() {
-  if (!registered_with_logging_service_)
-    return;
-  PasswordManagerInternalsService* service =
-      PasswordManagerInternalsServiceFactory::GetForBrowserContext(
-          Profile::FromWebUI(web_ui()));
-  if (service)
-    service->UnregisterReceiver(this);
+  UnregisterFromLoggingService();
+}
+
+void PasswordManagerInternalsUI::DidStartLoading(
+    content::RenderViewHost* /* render_view_host */) {
+  UnregisterFromLoggingService();
 }
 
 void PasswordManagerInternalsUI::DidStopLoading(
@@ -77,3 +76,13 @@ void PasswordManagerInternalsUI::LogSavePasswordProgress(
   web_ui()->CallJavascriptFunction("addSavePasswordProgressLog",
                                    text_string_value);
 }
+
+void PasswordManagerInternalsUI::UnregisterFromLoggingService() {
+  if (!registered_with_logging_service_)
+    return;
+  PasswordManagerInternalsService* service =
+      PasswordManagerInternalsServiceFactory::GetForBrowserContext(
+          Profile::FromWebUI(web_ui()));
+  if (service)
+    service->UnregisterReceiver(this);
+}
diff --git a/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui.h b/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui.h
index 2caea7b..2a51ee1 100644
--- a/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui.h
+++ b/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui.h
@@ -17,6 +17,8 @@ class PasswordManagerInternalsUI : public content::WebUIController,
   virtual ~PasswordManagerInternalsUI();
 
   // WebContentsObserver implementation.
+  virtual void DidStartLoading(
+      content::RenderViewHost* render_view_host) OVERRIDE;
   virtual void DidStopLoading(
       content::RenderViewHost* render_view_host) OVERRIDE;
 
@@ -24,6 +26,10 @@ class PasswordManagerInternalsUI : public content::WebUIController,
   virtual void LogSavePasswordProgress(const std::string& text) OVERRIDE;
 
  private:
+  // If currently registered with PasswordManagerInternalsService, unregisters
+  // |this|. Otherwise this is a no-op.
+  void UnregisterFromLoggingService();
+
   // Whether |this| registered as a log receiver with the
   // PasswordManagerInternalsService.
   bool registered_with_logging_service_;
diff --git a/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui_browsertest.cc b/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui_browsertest.cc
index 040e683..b424ac1 100644
--- a/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui_browsertest.cc
+++ b/chrome/browser/ui/webui/password_manager_internals/password_manager_internals_ui_browsertest.cc
@@ -25,9 +25,9 @@ class PasswordManagerInternalsWebUIBrowserTest : public WebUIBrowserTest {
  protected:
   content::WebContents* GetWebContents();
 
-  // Opens a new tab, and navigates to the internals page in that. Also assigns
-  // the corresponding UI controller to |controller_|.
-  void OpenNewTabWithTheInternalsPage();
+  // Navigates to the internals page in a tab specified by |disposition|. Also
+  // assigns the corresponding UI controller to |controller_|.
+  void OpenInternalsPage(WindowOpenDisposition disposition);
 
  private:
   PasswordManagerInternalsUI* controller_;
@@ -42,7 +42,7 @@ PasswordManagerInternalsWebUIBrowserTest::
 
 void PasswordManagerInternalsWebUIBrowserTest::SetUpOnMainThread() {
   WebUIBrowserTest::SetUpOnMainThread();
-  OpenNewTabWithTheInternalsPage();
+  OpenInternalsPage(CURRENT_TAB);
 }
 
 content::WebContents*
@@ -50,14 +50,14 @@ PasswordManagerInternalsWebUIBrowserTest::GetWebContents() {
   return browser()->tab_strip_model()->GetActiveWebContents();
 }
 
-void
-PasswordManagerInternalsWebUIBrowserTest::OpenNewTabWithTheInternalsPage() {
+void PasswordManagerInternalsWebUIBrowserTest::OpenInternalsPage(
+    WindowOpenDisposition disposition) {
   std::string url_string("chrome://");
   url_string += chrome::kChromeUIPasswordManagerInternalsHost;
   ui_test_utils::NavigateToURLWithDisposition(
       browser(),
       GURL(url_string),
-      NEW_FOREGROUND_TAB,
+      disposition,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
   controller_ = static_cast<PasswordManagerInternalsUI*>(
       GetWebContents()->GetWebUI()->GetController());
@@ -75,6 +75,18 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerInternalsWebUIBrowserTest,
   ASSERT_TRUE(RunJavascriptTest("testLogText"));
 }
 
+// Test that a single internals page is flushed on reload.
+IN_PROC_BROWSER_TEST_F(PasswordManagerInternalsWebUIBrowserTest,
+                       LogSavePasswordProgress_FlushedOnReload) {
+  password_manager::PasswordManagerInternalsService* service =
+      password_manager::PasswordManagerInternalsServiceFactory::
+          GetForBrowserContext(browser()->profile());
+  ASSERT_TRUE(service);
+  service->ProcessLog("<script> text for testing");
+  OpenInternalsPage(CURRENT_TAB);  // Reload.
+  ASSERT_TRUE(RunJavascriptTest("testLogTextNotPresent"));
+}
+
 // Test that if two tabs with the internals page are open, the second displays
 // the same logs. In particular, this checks that both the second tab gets the
 // logs created before the second tab was opened, and also that the second tab
@@ -90,7 +102,25 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerInternalsWebUIBrowserTest,
   service->ProcessLog("<script> text for testing");
   ASSERT_TRUE(RunJavascriptTest("testLogText"));
   // Now open a second tab with the internals page, but do not log anything.
-  OpenNewTabWithTheInternalsPage();
+  OpenInternalsPage(NEW_FOREGROUND_TAB);
   // The previously logged text should have made it to the page.
   ASSERT_TRUE(RunJavascriptTest("testLogText"));
 }
+
+// Test that in the presence of more internals pages, reload does not cause
+// flushing the logs.
+IN_PROC_BROWSER_TEST_F(PasswordManagerInternalsWebUIBrowserTest,
+                       LogSavePasswordProgress_NotFlushedOnReloadIfMultiple) {
+  // Open one more tab with the internals page.
+  OpenInternalsPage(NEW_FOREGROUND_TAB);
+  // Now log something.
+  password_manager::PasswordManagerInternalsService* service =
+      password_manager::PasswordManagerInternalsServiceFactory::
+          GetForBrowserContext(browser()->profile());
+  ASSERT_TRUE(service);
+  service->ProcessLog("<script> text for testing");
+  // Reload.
+  OpenInternalsPage(CURRENT_TAB);
+  // The text should still be there.
+  ASSERT_TRUE(RunJavascriptTest("testLogText"));
+}
diff --git a/chrome/test/data/webui/password_manager_internals_browsertest.js b/chrome/test/data/webui/password_manager_internals_browsertest.js
index dd3429c..310d021 100644
--- a/chrome/test/data/webui/password_manager_internals_browsertest.js
+++ b/chrome/test/data/webui/password_manager_internals_browsertest.js
@@ -12,3 +12,11 @@ function testLogText() {
                divLogs.innerHTML.match(/<script>/),
                "The logged text was not escaped.");
 }
+
+function testLogTextNotPresent() {
+  var divLogs = document.getElementById('log-entries');
+  assertNotEquals(null, divLogs, "The <div> with logs not found.");
+  assertEquals(null,
+               divLogs.innerHTML.match(/text for testing/),
+               "The logged text was found, but should not.");
+}