Commit a7dc4e82 authored by arthursonzogni's avatar arthursonzogni Committed by Commit Bot

PlzNavigate: Fix ServiceWorkerHandler not finding WebContents.

With PlzNavigate, the RenderFrameHost associated with a navigation is
known only at commit time. For this reason, in
ServiceWorkerHandler::OnWorkerVersionUpdated(), client's process_id(-1)
and route_id(-2) are invalid. They will be set to something meaningful
as soon as ServiceWorkerProviderHost::FinalizeInitialization will be
called with PlzNavigate, but it's too late.

The solution with PlzNavigate is to use a WebContentsGetter instead. In
this way, there is no need to know the RenderFrameHost.

BUG=725818
TEST=http/tests/inspector/service-workers/service-workers-redundant.html
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel

Review-Url: https://codereview.chromium.org/2917643002
Cr-Commit-Position: refs/heads/master@{#476331}
parent b239b006
...@@ -371,10 +371,13 @@ void ServiceWorkerHandler::OnWorkerVersionUpdated( ...@@ -371,10 +371,13 @@ void ServiceWorkerHandler::OnWorkerVersionUpdated(
protocol::Array<std::string>::create(); protocol::Array<std::string>::create();
for (const auto& client : version.clients) { for (const auto& client : version.clients) {
if (client.second.type == SERVICE_WORKER_PROVIDER_FOR_WINDOW) { if (client.second.type == SERVICE_WORKER_PROVIDER_FOR_WINDOW) {
RenderFrameHostImpl* render_frame_host = RenderFrameHostImpl::FromID( // PlzNavigate: a navigation may not yet be associated with a
client.second.process_id, client.second.route_id); // RenderFrameHost. Use the |web_contents_getter| instead.
WebContents* web_contents = WebContents* web_contents =
WebContents::FromRenderFrameHost(render_frame_host); client.second.web_contents_getter
? client.second.web_contents_getter.Run()
: WebContents::FromRenderFrameHost(RenderFrameHostImpl::FromID(
client.second.process_id, client.second.route_id));
// There is a possibility that the frame is already deleted // There is a possibility that the frame is already deleted
// because of the thread hopping. // because of the thread hopping.
if (!web_contents) if (!web_contents)
......
...@@ -846,11 +846,11 @@ void ServiceWorkerContextCore::OnControlleeAdded( ...@@ -846,11 +846,11 @@ void ServiceWorkerContextCore::OnControlleeAdded(
ServiceWorkerProviderHost* provider_host) { ServiceWorkerProviderHost* provider_host) {
if (!observer_list_) if (!observer_list_)
return; return;
observer_list_->Notify(FROM_HERE, observer_list_->Notify(
&ServiceWorkerContextObserver::OnControlleeAdded, FROM_HERE, &ServiceWorkerContextObserver::OnControlleeAdded,
version->version_id(), provider_host->client_uuid(), version->version_id(), provider_host->client_uuid(),
provider_host->process_id(), provider_host->route_id(), provider_host->process_id(), provider_host->route_id(),
provider_host->provider_type()); provider_host->web_contents_getter(), provider_host->provider_type());
} }
void ServiceWorkerContextCore::OnControlleeRemoved( void ServiceWorkerContextCore::OnControlleeRemoved(
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <stdint.h> #include <stdint.h>
#include "base/callback.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "content/browser/service_worker/service_worker_info.h" #include "content/browser/service_worker/service_worker_info.h"
...@@ -72,11 +73,14 @@ class ServiceWorkerContextObserver { ...@@ -72,11 +73,14 @@ class ServiceWorkerContextObserver {
int process_id, int process_id,
int thread_id, int thread_id,
const ConsoleMessage& message) {} const ConsoleMessage& message) {}
virtual void OnControlleeAdded(int64_t version_id, // |web_contents_getter| is only set in PlzNavigate.
const std::string& uuid, virtual void OnControlleeAdded(
int process_id, int64_t version_id,
int route_id, const std::string& uuid,
ServiceWorkerProviderType type) {} int process_id,
int route_id,
const base::Callback<WebContents*(void)>& web_contents_getter,
ServiceWorkerProviderType type) {}
virtual void OnControlleeRemoved(int64_t version_id, virtual void OnControlleeRemoved(int64_t version_id,
const std::string& uuid) {} const std::string& uuid) {}
virtual void OnRegistrationStored(int64_t registration_id, virtual void OnRegistrationStored(int64_t registration_id,
......
...@@ -276,13 +276,14 @@ void ServiceWorkerContextWatcher::OnControlleeAdded( ...@@ -276,13 +276,14 @@ void ServiceWorkerContextWatcher::OnControlleeAdded(
const std::string& uuid, const std::string& uuid,
int process_id, int process_id,
int route_id, int route_id,
const base::Callback<WebContents*(void)>& web_contents_getter,
ServiceWorkerProviderType type) { ServiceWorkerProviderType type) {
auto it = version_info_map_.find(version_id); auto it = version_info_map_.find(version_id);
if (it == version_info_map_.end()) if (it == version_info_map_.end())
return; return;
ServiceWorkerVersionInfo* version = it->second.get(); ServiceWorkerVersionInfo* version = it->second.get();
version->clients[uuid] = version->clients[uuid] = ServiceWorkerVersionInfo::ClientInfo(
ServiceWorkerVersionInfo::ClientInfo(process_id, route_id, type); process_id, route_id, web_contents_getter, type);
SendVersionInfo(*version); SendVersionInfo(*version);
} }
......
...@@ -90,11 +90,13 @@ class ServiceWorkerContextWatcher ...@@ -90,11 +90,13 @@ class ServiceWorkerContextWatcher
int process_id, int process_id,
int thread_id, int thread_id,
const ConsoleMessage& message) override; const ConsoleMessage& message) override;
void OnControlleeAdded(int64_t version_id, void OnControlleeAdded(
const std::string& uuid, int64_t version_id,
int process_id, const std::string& uuid,
int route_id, int process_id,
ServiceWorkerProviderType type) override; int route_id,
const base::Callback<WebContents*(void)>& web_contents_getter,
ServiceWorkerProviderType type) override;
void OnControlleeRemoved(int64_t version_id, void OnControlleeRemoved(int64_t version_id,
const std::string& uuid) override; const std::string& uuid) override;
void OnRegistrationStored(int64_t registration_id, void OnRegistrationStored(int64_t registration_id,
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "content/browser/service_worker/embedded_worker_status.h" #include "content/browser/service_worker/embedded_worker_status.h"
#include "content/common/service_worker/service_worker_types.h" #include "content/common/service_worker/service_worker_types.h"
#include "content/public/common/browser_side_navigation_policy.h"
#include "content/public/common/child_process_host.h" #include "content/public/common/child_process_host.h"
#include "ipc/ipc_message.h" #include "ipc/ipc_message.h"
...@@ -14,13 +15,21 @@ namespace content { ...@@ -14,13 +15,21 @@ namespace content {
ServiceWorkerVersionInfo::ClientInfo::ClientInfo() ServiceWorkerVersionInfo::ClientInfo::ClientInfo()
: ClientInfo(ChildProcessHost::kInvalidUniqueID, : ClientInfo(ChildProcessHost::kInvalidUniqueID,
MSG_ROUTING_NONE, MSG_ROUTING_NONE,
base::Callback<WebContents*(void)>(),
SERVICE_WORKER_PROVIDER_UNKNOWN) {} SERVICE_WORKER_PROVIDER_UNKNOWN) {}
ServiceWorkerVersionInfo::ClientInfo::ClientInfo(int process_id, ServiceWorkerVersionInfo::ClientInfo::ClientInfo(
int route_id, int process_id,
ServiceWorkerProviderType type) int route_id,
: process_id(process_id), route_id(route_id), type(type) { const base::Callback<WebContents*(void)>& web_contents_getter,
} ServiceWorkerProviderType type)
: process_id(process_id),
route_id(route_id),
web_contents_getter(web_contents_getter),
type(type) {}
ServiceWorkerVersionInfo::ClientInfo::ClientInfo(
const ServiceWorkerVersionInfo::ClientInfo& other) = default;
ServiceWorkerVersionInfo::ClientInfo::~ClientInfo() { ServiceWorkerVersionInfo::ClientInfo::~ClientInfo() {
} }
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <vector> #include <vector>
#include "base/callback.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "content/browser/service_worker/service_worker_version.h" #include "content/browser/service_worker/service_worker_version.h"
#include "content/common/content_export.h" #include "content/common/content_export.h"
...@@ -24,10 +25,16 @@ struct CONTENT_EXPORT ServiceWorkerVersionInfo { ...@@ -24,10 +25,16 @@ struct CONTENT_EXPORT ServiceWorkerVersionInfo {
struct CONTENT_EXPORT ClientInfo { struct CONTENT_EXPORT ClientInfo {
public: public:
ClientInfo(); ClientInfo();
ClientInfo(int process_id, int route_id, ServiceWorkerProviderType type); ClientInfo(int process_id,
int route_id,
const base::Callback<WebContents*(void)>& web_contents_getter,
ServiceWorkerProviderType type);
ClientInfo(const ClientInfo& other);
~ClientInfo(); ~ClientInfo();
int process_id; int process_id;
int route_id; int route_id;
// |web_contents_getter| is only set for PlzNavigate.
base::Callback<WebContents*(void)> web_contents_getter;
ServiceWorkerProviderType type; ServiceWorkerProviderType type;
}; };
......
...@@ -369,7 +369,8 @@ ServiceWorkerVersionInfo ServiceWorkerVersion::GetInfo() { ...@@ -369,7 +369,8 @@ ServiceWorkerVersionInfo ServiceWorkerVersion::GetInfo() {
info.clients.insert(std::make_pair( info.clients.insert(std::make_pair(
host->client_uuid(), host->client_uuid(),
ServiceWorkerVersionInfo::ClientInfo( ServiceWorkerVersionInfo::ClientInfo(
host->process_id(), host->route_id(), host->provider_type()))); host->process_id(), host->route_id(), host->web_contents_getter(),
host->provider_type())));
} }
if (!main_script_http_info_) if (!main_script_http_info_)
return info; return info;
......
...@@ -25,10 +25,6 @@ Bug(718942) virtual/mojo-loading/http/tests/security/contentSecurityPolicy/1.1/f ...@@ -25,10 +25,6 @@ Bug(718942) virtual/mojo-loading/http/tests/security/contentSecurityPolicy/1.1/f
Bug(718942) virtual/mojo-loading/http/tests/security/contentSecurityPolicy/1.1/form-action-src-redirect-blocked.html [ Failure ] Bug(718942) virtual/mojo-loading/http/tests/security/contentSecurityPolicy/1.1/form-action-src-redirect-blocked.html [ Failure ]
Bug(718942) virtual/off-main-thread-fetch/http/tests/misc/onload-detach-during-csp-frame-src-none.html [ Failure ] Bug(718942) virtual/off-main-thread-fetch/http/tests/misc/onload-detach-during-csp-frame-src-none.html [ Failure ]
# PlzNavigate: A few line are missing in InspectorTest.dumpServiceWorkersView()
Bug(725818) http/tests/inspector/service-workers/service-workers-redundant.html [ Failure ]
Bug(725818) virtual/off-main-thread-fetch/http/tests/inspector/service-workers/service-workers-redundant.html [ Failure ]
# PlzNavigate: Navigation requests upgraded via upgrade-insecure-requests will not get reported # PlzNavigate: Navigation requests upgraded via upgrade-insecure-requests will not get reported
# See https://crbug.com/713388 # See https://crbug.com/713388
Bug(713388) external/wpt/content-security-policy/securitypolicyviolation/upgrade-insecure-requests-reporting.https.html [ Timeout ] Bug(713388) external/wpt/content-security-policy/securitypolicyviolation/upgrade-insecure-requests-reporting.https.html [ Timeout ]
......
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