Don't call WebCacheManager::Remove(RPH_id) to clear webview's cache.

We already call WebCacheManager::ClearCacheForProcess(RPH_id), which seems to be sufficient in my local testing. I remember that clearing webview cache didn't seem to work without calling WCM::Remove() when I implemented it. After r380598, Calling WCM::Remove also deletes mojo service associated with the RPH, so the next call to WCM::Remove with the same RPH will result in a nullptr access. BUG=615429 Test=Added a test, see http://crbug.com/615429#c2 for repro. Review-Url: https://codereview.chromium.org/2033993002 Cr-Commit-Position: refs/heads/master@{#397803}

Don't call WebCacheManager::Remove(RPH_id) to clear webview's cache.
We already call WebCacheManager::ClearCacheForProcess(RPH_id), which seems to be sufficient in my local testing. I remember that clearing webview cache didn't seem to work without calling WCM::Remove() when I implemented it. After r380598, Calling WCM::Remove also deletes mojo service associated with the RPH, so the next call to WCM::Remove with the same RPH will result in a nullptr access. BUG=615429 Test=Added a test, see http://crbug.com/615429#c2 for repro. Review-Url: https://codereview.chromium.org/2033993002 Cr-Commit-Position: refs/heads/master@{#397803}
d0dae71f · lazyboy · Commit bot · ca2f5499 · d0dae71f · d0dae71f
Commit d0dae71f authored Jun 03, 2016 by lazyboy Committed by Commit bot Jun 03, 2016
4 changed files
--- a/chrome/browser/apps/guest_view/web_view_browsertest.cc
+++ b/chrome/browser/apps/guest_view/web_view_browsertest.cc
@@ -2268,6 +2268,14 @@ IN_PROC_BROWSER_TEST_P(WebViewTest, ClearData) {
          << message_;
 }

+// Regression test for https://crbug.com/615429.
+IN_PROC_BROWSER_TEST_P(WebViewTest, ClearDataTwice) {
+  ASSERT_TRUE(StartEmbeddedTestServer());  // For serving guest pages.
+  ASSERT_TRUE(RunPlatformAppTestWithArg("platform_apps/web_view/common",
+                                        "cleardata_twice"))
+      << message_;
+}
+
 #if defined(OS_WIN)
 // Test is disabled on Windows because it fails often (~9% time)
 // http://crbug.com/489088

--- a/chrome/test/data/extensions/platform_apps/web_view/common/cleardata_twice/bootstrap.js
+++ b/chrome/test/data/extensions/platform_apps/web_view/common/cleardata_twice/bootstrap.js
+// Copyright 2016 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.
+
+config.IS_CHROME_TEST = true;
+// Guest served from TestServer.
+config.IS_JS_ONLY_GUEST = false;
+config.TEST_DIR = 'cleardata_twice';
+
+var clearDataTwiceTests = {};
+
+var run = function() {
+  var container = document.createElement('div');
+  container.id = 'webview-tag-container';
+  document.body.appendChild(container);
+
+  chrome.test.getConfig(function(chromeConfig) {
+    window.console.log('getConfig: ' + chromeConfig);
+    utils.setUp(chromeConfig, config);
+    embedder.loadGuest(function() {
+      chrome.test.runTests([
+        clearDataTwiceTests.clear
+      ]);
+    }, function(data) {
+      // We don't expect guest to send us any postMessage.
+      chrome.test.fail();
+    });
+  });
+};
+
+// Tests.
+// Clears http cache of a webview twice.
+clearDataTwiceTests.clear = function testClearDataTwice() {
+  console.log('clearDataTwiceTests.clear');
+  embedder.webview.clearData({}, {cache: true}, function() {
+    embedder.webview.clearData({}, {cache: true}, function() {
+      chrome.test.succeed();
+    });
+  });
+};
+
+// Run test(s).
+run();
--- a/chrome/test/data/extensions/platform_apps/web_view/common/cleardata_twice/guest.html
+++ b/chrome/test/data/extensions/platform_apps/web_view/common/cleardata_twice/guest.html
+<!doctype html>
+<!--
+ * Copyright 2016 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.
+-->
+<html>
+  <head>
+    <script type="text/javascript">
+      // A guest that stores and deletes cookies.
+      // Note that the embedder has to initiate a postMessage first so that
+      // the guest has a reference to the embedder's window.
+
+      // The window reference of the embedder to send post message reply.
+      var embedderWindowChannel = null;
+
+      // A value that uniquely identifies the guest sending the messages to the
+      // embedder.
+      var channelId = 0;
+      var notifyEmbedder = function(msg_array) {
+        var msg = msg_array.concat([channelId]);
+        embedderWindowChannel.postMessage(JSON.stringify(msg), '*');
+      };
+
+      var onPostMessageReceived = function(e) {
+        embedderWindowChannel = e.source;
+        var data = JSON.parse(e.data);
+        if (data[0] == 'create-channel') {
+          window.console.log('guest: create-channel');
+          channelId = data[1];
+          notifyEmbedder(['channel-created']);
+          return;
+        }
+      };
+      window.addEventListener('message', onPostMessageReceived, false);
+    </script>
+  </head>
+  <body>
+    <div>Simple guest page.</div>
+  </body>
+</html>
--- a/extensions/browser/guest_view/web_view/web_view_guest.cc
+++ b/extensions/browser/guest_view/web_view/web_view_guest.cc
@@ -750,7 +750,6 @@ bool WebViewGuest::ClearData(base::Time remove_since,
    // StoragePartitionHttpCacheDataRemover::ClearData() does not clear that.
    web_cache::WebCacheManager::GetInstance()->ClearCacheForProcess(
        render_process_id);
-    web_cache::WebCacheManager::GetInstance()->Remove(render_process_id);

    base::Closure cache_removal_done_callback = base::Bind(
        &WebViewGuest::ClearDataInternal, weak_ptr_factory_.GetWeakPtr(),