Skip to content

GitLab

T
tangled
Commit 3cae1851 authored by nasko's avatar nasko Committed by Commit bot
Browse files
Options

Revert of Fix event page process cleanup when running in --isolate-extensions... 

Revert of Fix event page process cleanup when running in --isolate-extensions mode. (patchset #6 id:100001 of https://codereview.chromium.org/2028873002/ )

Reason for revert:
Speculative revert of this CL to check if it is the root cause of https://crbug.com/616989.

Original issue's description:
> Fix event page process cleanup when running in --isolate-extensions mode.
>
> With --isolate-extensions, navigations can start in one RenderFrameHost
> and complete in another. This was not possible to happen with extensions
> related RenderFrameHosts in the past, so the code wasn't able to handle
> it. This CL refactors the OnNetworkRequest(Started|Stopped) to use the
> navigation request id as the stable identifier instead of the associated
> RenderFrameHost.
>
> BUG=612668
>
> Committed: https://crrev.com/c25500369dd9a7f49d2b7d276556b2e22ba840c8
> Cr-Commit-Position: refs/heads/master@{#397328}

TBR=rdevlin.cronin@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=612668

Review-Url: https://codereview.chromium.org/2044643003
Cr-Commit-Position: refs/heads/master@{#398185}
parent 5824d790
Hide whitespace changes
Inline Side-by-side
Showing with 15 additions and 81 deletions
chrome/browser/extensions/lazy_background_page_apitest.cc
View file @ 3cae1851
......@@ -37,7 +37,6 @@
#include "extensions/browser/extension_registry_observer.h"
#include "extensions/browser/process_manager.h"
#include "extensions/common/extension.h"
#include "extensions/common/switches.h"
#include "extensions/test/extension_test_message_listener.h"
#include "extensions/test/result_catcher.h"
#include "net/dns/mock_host_resolver.h"
......@@ -105,8 +104,8 @@ class LazyBackgroundPageApiTest : public ExtensionApiTest {
ExtensionApiTest::SetUpCommandLine(command_line);
// Disable background network activity as it can suddenly bring the Lazy
// Background Page alive.
command_line->AppendSwitch(::switches::kDisableBackgroundNetworking);
command_line->AppendSwitch(::switches::kNoProxyServer);
command_line->AppendSwitch(switches::kDisableBackgroundNetworking);
command_line->AppendSwitch(switches::kNoProxyServer);
}
// Loads the extension, which temporarily starts the lazy background page
......@@ -660,39 +659,4 @@ IN_PROC_BROWSER_TEST_F(LazyBackgroundPageApiTest, OnSuspendUseStorageApi) {
// TODO: background page with timer.
// TODO: background page that interacts with popup.
// Test class to allow test cases to run in --isolate-extensions mode.
class LazyBackgroundPageIsolatedExtensionsApiTest
: public LazyBackgroundPageApiTest {
public:
LazyBackgroundPageIsolatedExtensionsApiTest() {}
~LazyBackgroundPageIsolatedExtensionsApiTest() override {}
void SetUpInProcessBrowserTestFixture() override {
LazyBackgroundPageApiTest::SetUpInProcessBrowserTestFixture();
// This is needed to allow example.com to actually resolve and load in
// tests.
host_resolver()->AddRule("*", "127.0.0.1");
}
void SetUpCommandLine(base::CommandLine* command_line) override {
LazyBackgroundPageApiTest::SetUpCommandLine(command_line);
command_line->AppendSwitch(switches::kIsolateExtensions);
}
private:
DISALLOW_COPY_AND_ASSIGN(LazyBackgroundPageIsolatedExtensionsApiTest);
};
// Ensure that the events page of an extension is properly torn down and the
// process does not linger around when running in --isolate-extensions mode.
// See https://crbug.com/612668.
IN_PROC_BROWSER_TEST_F(LazyBackgroundPageIsolatedExtensionsApiTest,
EventProcessCleanup) {
ASSERT_TRUE(LoadExtensionAndWait("event_page_with_web_iframe"));
// Lazy Background Page doesn't exist anymore.
EXPECT_FALSE(IsBackgroundPageAlive(last_loaded_extension_id()));
}
} // namespace extensions
chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/event_page.html deleted 100644 → 0
View file @ 5824d790
<!doctype html>
<meta charset="utf-8">
<title>Event page with web iframe</title>
<iframe src="http://example.com"></iframe>
chrome/test/data/extensions/api_test/lazy_background_page/event_page_with_web_iframe/manifest.json deleted 100644 → 0
View file @ 5824d790
{
"name": "Event page with web iframe",
"version": "1.0",
"background": {
"page": "event_page.html",
"persistent": false
},
"permissions": [
"activeTab"
],
"manifest_version": 2
}
extensions/browser/process_manager.cc
View file @ 3cae1851
......@@ -570,37 +570,24 @@ void ProcessManager::OnNetworkRequestStarted(
uint64_t request_id) {
ExtensionHost* host = GetBackgroundHostForExtension(
GetExtensionID(render_frame_host));
if (!host || !IsFrameInExtensionHost(host, render_frame_host))
return;
auto result =
pending_network_requests_.insert(std::make_pair(request_id, host));
auto result = pending_network_requests_.insert(request_id);
DCHECK(result.second) << "Duplicate network request IDs.";
IncrementLazyKeepaliveCount(host->extension());
host->OnNetworkRequestStarted(request_id);
if (host && IsFrameInExtensionHost(host, render_frame_host)) {
IncrementLazyKeepaliveCount(host->extension());
host->OnNetworkRequestStarted(request_id);
}
}
void ProcessManager::OnNetworkRequestDone(
content::RenderFrameHost* render_frame_host,
uint64_t request_id) {
auto result = pending_network_requests_.find(request_id);
if (result == pending_network_requests_.end())
return;
// The cached |host| can be invalid, if it was deleted between the time it
// was inserted in the map and the look up. It is checked to ensure it is in
// the list of existing background_hosts_.
ExtensionHost* host = result->second;
pending_network_requests_.erase(result);
if (background_hosts_.find(host) == background_hosts_.end())
return;
DCHECK(IsFrameInExtensionHost(host, render_frame_host));
host->OnNetworkRequestDone(request_id);
DecrementLazyKeepaliveCount(host->extension());
ExtensionHost* host = GetBackgroundHostForExtension(
GetExtensionID(render_frame_host));
if (host && IsFrameInExtensionHost(host, render_frame_host)) {
host->OnNetworkRequestDone(request_id);
if (pending_network_requests_.erase(request_id))
DecrementLazyKeepaliveCount(host->extension());
}
}
void ProcessManager::CancelSuspend(const Extension* extension) {
......
extensions/browser/process_manager.h
View file @ 3cae1851
......@@ -326,7 +326,7 @@ class ProcessManager : public KeyedService,
// extension URLRequest is constructed and then destroyed without ever
// starting, we can receive a completion notification without a corresponding
// start notification. In that case we want to avoid decrementing keepalive.
std::map<int, ExtensionHost*> pending_network_requests_;
std::set<int> pending_network_requests_;
// Must be last member, see doc on WeakPtrFactory.
base::WeakPtrFactory<ProcessManager> weak_ptr_factory_;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment