Fix crash as a result of my previous change to fix the memory leak in WebCacheManager.

BUG=12362
TEST=added regression test
Review URL: http://codereview.chromium.org/113754

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

7 files changed, 90 insertions, 11 deletions

diff --git a/chrome/browser/child_process_security_policy_browser_test.cc b/chrome/browser/child_process_security_policy_browser_test.cc
index 2e3e1c1..ab613f3 100644
--- a/chrome/browser/child_process_security_policy_browser_test.cc
+++ b/chrome/browser/child_process_security_policy_browser_test.cc
@@ -10,10 +10,8 @@
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/child_process_security_policy.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
-#include "chrome/common/chrome_paths.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
-#include "net/base/net_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 class ChildProcessSecurityPolicyInProcessBrowserTest
@@ -33,11 +31,7 @@ class ChildProcessSecurityPolicyInProcessBrowserTest
 };
 
 IN_PROC_BROWSER_TEST_F(ChildProcessSecurityPolicyInProcessBrowserTest, NoLeak) {
-  FilePath path;
-  PathService::Get(chrome::DIR_TEST_DATA, &path);
-  path = path.Append(FilePath::FromWStringHack(L"google"));
-  path = path.Append(FilePath::FromWStringHack(L"google.html"));
-  GURL url = net::FilePathToFileURL(path);
+  GURL url(ui_test_utils::GetTestUrl(L"google", L"google.html"));
 
   ui_test_utils::NavigateToURL(browser(), url);
   EXPECT_EQ(
diff --git a/chrome/browser/renderer_host/web_cache_manager.cc b/chrome/browser/renderer_host/web_cache_manager.cc
index 4615e2b..3135ae4 100644
--- a/chrome/browser/renderer_host/web_cache_manager.cc
+++ b/chrome/browser/renderer_host/web_cache_manager.cc
@@ -95,12 +95,13 @@ void WebCacheManager::Remove(int renderer_id) {
 }
 
 void WebCacheManager::ObserveActivity(int renderer_id) {
+  StatsMap::iterator item = stats_.find(renderer_id);
+  if (item == stats_.end())
+    return;  // We might see stats for a renderer that has been destroyed.
+
   // Record activity.
   active_renderers_.insert(renderer_id);
-
-  StatsMap::iterator item = stats_.find(renderer_id);
-  if (item != stats_.end())
-    item->second.access = Time::Now();
+  item->second.access = Time::Now();
 
   std::set<int>::iterator elmt = inactive_renderers_.find(renderer_id);
   if (elmt != inactive_renderers_.end()) {
diff --git a/chrome/browser/renderer_host/web_cache_manager.h b/chrome/browser/renderer_host/web_cache_manager.h
index 70f3560..8de934f 100644
--- a/chrome/browser/renderer_host/web_cache_manager.h
+++ b/chrome/browser/renderer_host/web_cache_manager.h
@@ -17,6 +17,7 @@
 #include "base/singleton.h"
 #include "base/task.h"
 #include "base/time.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"
 #include "webkit/api/public/WebCache.h"
 
 class PrefService;
@@ -24,6 +25,7 @@ class PrefService;
 class WebCacheManager {
   // Unit tests are our friends.
   friend class WebCacheManagerTest;
+  FRIEND_TEST(WebCacheManagerBrowserTest, CrashOnceOnly);
 
  public:
   static void RegisterPrefs(PrefService* prefs);
diff --git a/chrome/browser/renderer_host/web_cache_manager_browser_test.cc b/chrome/browser/renderer_host/web_cache_manager_browser_test.cc
new file mode 100644
index 0000000..d3826a6
--- /dev/null
+++ b/chrome/browser/renderer_host/web_cache_manager_browser_test.cc
@@ -0,0 +1,60 @@
+// Copyright (c) 2009 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 <string>
+
+#include "base/message_loop.h"
+#include "chrome/browser/browser.h"
+#include "chrome/browser/renderer_host/render_process_host.h"
+#include "chrome/browser/renderer_host/web_cache_manager.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/test/in_process_browser_test.h"
+#include "chrome/test/ui_test_utils.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class WebCacheManagerBrowserTest : public InProcessBrowserTest {
+};
+
+// Regression test for http://crbug.com/12362.  If a renderer crashes and the
+// user navigates to another tab and back, the browser doesn't crash.
+IN_PROC_BROWSER_TEST_F(WebCacheManagerBrowserTest, CrashOnceOnly) {
+  GURL url(ui_test_utils::GetTestUrl(L"google", L"google.html"));
+
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  browser()->NewTab();
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  TabContents* tab = browser()->GetTabContentsAt(0);
+  ASSERT_TRUE(tab != NULL);
+  base::KillProcess(
+      tab->process()->process().handle(), base::PROCESS_END_KILLED_BY_USER,
+      true);
+
+  browser()->SelectTabContentsAt(0, true);
+  browser()->NewTab();
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  browser()->SelectTabContentsAt(0, true);
+  browser()->NewTab();
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // We would have crashed at the above line with the bug.
+
+  browser()->SelectTabContentsAt(0, true);
+  browser()->CloseTab();
+  browser()->SelectTabContentsAt(0, true);
+  browser()->CloseTab();
+  browser()->SelectTabContentsAt(0, true);
+  browser()->CloseTab();
+
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  EXPECT_EQ(
+      WebCacheManager::GetInstance()->active_renderers_.size(), 1U);
+  EXPECT_EQ(
+      WebCacheManager::GetInstance()->inactive_renderers_.size(), 0U);
+  EXPECT_EQ(
+      WebCacheManager::GetInstance()->stats_.size(), 1U);
+}
diff --git a/chrome/test/browser/browser_tests_dll.vcproj b/chrome/test/browser/browser_tests_dll.vcproj
index 9f9a321..553b4b5 100644
--- a/chrome/test/browser/browser_tests_dll.vcproj
+++ b/chrome/test/browser/browser_tests_dll.vcproj
@@ -314,6 +314,14 @@
 				>

 			</File>

 		</Filter>

+		<Filter

+			Name="TestWebCacheManager"

+			>

+			<File

+				RelativePath="..\..\browser\renderer_host\web_cache_manager_browser_test.cc"

+				>

+			</File>

+		</Filter>

 	</Files>

 	<Globals>

 	</Globals>

diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
index 2e92105..1f252a0 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -6,13 +6,16 @@
 
 #include "base/json_reader.h"
 #include "base/message_loop.h"
+#include "base/path_service.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/dom_operation_notification_details.h"
 #include "chrome/browser/tab_contents/navigation_controller.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/notification_service.h"
 #include "googleurl/src/gurl.h"
+#include "net/base/net_util.h"
 #include "views/widget/accelerator_handler.h"
 
 namespace ui_test_utils {
@@ -189,4 +192,13 @@ bool ExecuteJavaScriptAndExtractString(TabContents* tab_contents,
 
   return value->GetAsString(result);
 }
+
+GURL GetTestUrl(const std::wstring& dir, const std::wstring file) {
+  FilePath path;
+  PathService::Get(chrome::DIR_TEST_DATA, &path);
+  path = path.Append(FilePath::FromWStringHack(dir));
+  path = path.Append(FilePath::FromWStringHack(file));
+  return net::FilePathToFileURL(path);
+}
+
 }  // namespace ui_test_utils
diff --git a/chrome/test/ui_test_utils.h b/chrome/test/ui_test_utils.h
index 3c166e8..c25356e 100644
--- a/chrome/test/ui_test_utils.h
+++ b/chrome/test/ui_test_utils.h
@@ -70,6 +70,8 @@ bool ExecuteJavaScriptAndExtractString(TabContents* tab_contents,
                                        const std::wstring& frame_xpath,
                                        const std::wstring& script,
                                        std::string* result);
+
+GURL GetTestUrl(const std::wstring& dir, const std::wstring file);
 }
 
 #endif  // CHROME_TEST_UI_TEST_UTILS_H_