Skip to content

Commit

Permalink
M63: Revert "[ServiceWorker] Implement ServiceWorkerContainer.SetCont…
Browse files Browse the repository at this point in the history 
…roller"

This reverts commit 24e5c55.

Bug: 774681
Change-Id: I2624c46835a32d60a7361077b3bbe20939faa307
TBR: kinuko
Reviewed-on: https://chromium-review.googlesource.com/728879
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#85}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
  • Loading branch information
@mfalken
mfalken committed Oct 19, 2017
1 parent 860a95c commit 75baffd
Show file tree
Hide file tree
Showing 16 changed files with 315 additions and 368 deletions.
36 changes: 15 additions & 21 deletions content/browser/service_worker/service_worker_provider_host.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -559,13 +559,6 @@ ServiceWorkerProviderHost::PrepareForCrossSiteTransfer() {
DCHECK_EQ(kDocumentMainThreadId, render_thread_id_);
DCHECK_NE(SERVICE_WORKER_PROVIDER_UNKNOWN, info_.type);

// Clear the controller from the renderer-side provider, since no one knows
// what's going to happen until after cross-site transfer finishes.
if (controller_) {
SendSetControllerServiceWorker(nullptr,
false /* notify_controllerchange */);
}

std::unique_ptr<ServiceWorkerProviderHost> provisional_host =
base::WrapUnique(new ServiceWorkerProviderHost(
process_id(),
Expand All @@ -578,6 +571,13 @@ ServiceWorkerProviderHost::PrepareForCrossSiteTransfer() {

RemoveAllMatchingRegistrations();

// Clear the controller from the renderer-side provider, since no one knows
// what's going to happen until after cross-site transfer finishes.
if (controller_) {
SendSetControllerServiceWorker(nullptr,
false /* notify_controllerchange */);
}

render_process_id_ = ChildProcessHost::kInvalidUniqueID;
render_thread_id_ = kInvalidEmbeddedWorkerThreadId;
dispatcher_host_ = nullptr;
Expand Down Expand Up @@ -881,20 +881,14 @@ void ServiceWorkerProviderHost::SendSetControllerServiceWorker(
DCHECK_EQ(controller_.get(), version);
}

std::vector<blink::mojom::WebFeature> used_features;
if (version) {
for (const auto& feature : version->used_features()) {
// TODO: version->used_features() should never have a feature outside the
// known feature range. But there is special case, see the details in
// crbug.com/758419.
if (feature >= 0 &&
feature < static_cast<uint32_t>(
blink::mojom::WebFeature::kNumberOfFeatures))
used_features.push_back(static_cast<blink::mojom::WebFeature>(feature));
}
}
container_->SetController(GetOrCreateServiceWorkerHandle(version),
used_features, notify_controllerchange);
ServiceWorkerMsg_SetControllerServiceWorker_Params params;
params.thread_id = render_thread_id_;
params.provider_id = provider_id();
params.object_info = GetOrCreateServiceWorkerHandle(version);
params.should_notify_controllerchange = notify_controllerchange;
if (version)
params.used_features = version->used_features();
Send(new ServiceWorkerMsg_SetControllerServiceWorker(params));
}

void ServiceWorkerProviderHost::Register(
Expand Down
63 changes: 23 additions & 40 deletions content/browser/service_worker/service_worker_provider_host_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ namespace content {

namespace {

// Sets the document URL for |host| and associates it with |registration|.
// A dumb version of
// ServiceWorkerControlleeRequestHandler::PrepareForMainResource().
void SimulateServiceWorkerControlleeRequestHandler(
ServiceWorkerProviderHost* host,
ServiceWorkerRegistration* registration) {
host->SetDocumentUrl(GURL("https://www.example.com/page"));
host->AssociateRegistration(registration,
false /* notify_controllerchange */);
}

const char kServiceWorkerScheme[] = "i-can-use-service-worker";

class ServiceWorkerTestContentClient : public TestContentClient {
Expand Down Expand Up @@ -361,27 +372,6 @@ TEST_F(ServiceWorkerProviderHostTest, RemoveProvider) {
EXPECT_FALSE(context_->GetProviderHost(process_id, provider_id));
}

class MockServiceWorkerContainer : public mojom::ServiceWorkerContainer {
public:
explicit MockServiceWorkerContainer(
mojom::ServiceWorkerContainerAssociatedRequest request)
: binding_(this, std::move(request)) {}

~MockServiceWorkerContainer() override = default;

void SetController(const ServiceWorkerObjectInfo& controller,
const std::vector<blink::mojom::WebFeature>& used_features,
bool should_notify_controllerchange) override {
was_set_controller_called_ = true;
}

bool was_set_controller_called() const { return was_set_controller_called_; }

private:
bool was_set_controller_called_ = false;
mojo::AssociatedBinding<mojom::ServiceWorkerContainer> binding_;
};

TEST_F(ServiceWorkerProviderHostTest, Controller) {
base::CommandLine::ForCurrentProcess()->AppendSwitch(
switches::kEnableBrowserSideNavigation);
Expand All @@ -395,31 +385,27 @@ TEST_F(ServiceWorkerProviderHostTest, Controller) {
true /* is_parent_frame_secure */);
remote_endpoints_.emplace_back();
remote_endpoints_.back().BindWithProviderHostInfo(&info);
auto container = base::MakeUnique<MockServiceWorkerContainer>(
std::move(*remote_endpoints_.back().client_request()));

// Create an active version and then start the navigation.
scoped_refptr<ServiceWorkerVersion> version = new ServiceWorkerVersion(
registration1_.get(), GURL("https://www.example.com/sw.js"),
1 /* version_id */, helper_->context()->AsWeakPtr());
registration1_->SetActiveVersion(version);
SimulateServiceWorkerControlleeRequestHandler(host.get(),
registration1_.get());

// Finish the navigation.
host->SetDocumentUrl(GURL("https://www.example.com/page"));
host->CompleteNavigationInitialized(
helper_->mock_render_process_id(), std::move(info),
helper_->GetDispatcherHostForProcess(helper_->mock_render_process_id())
->AsWeakPtr());

host->AssociateRegistration(registration1_.get(),
false /* notify_controllerchange */);
base::RunLoop().RunUntilIdle();

// The page should be controlled since there was an active version at the
// time navigation started. The SetController IPC should have been sent.
EXPECT_TRUE(host->active_version());
EXPECT_EQ(host->active_version(), host->controller());
EXPECT_TRUE(container->was_set_controller_called());
EXPECT_TRUE(helper_->ipc_sink()->GetUniqueMessageMatching(
ServiceWorkerMsg_SetControllerServiceWorker::ID));
}

TEST_F(ServiceWorkerProviderHostTest, ActiveIsNotController) {
Expand All @@ -435,34 +421,31 @@ TEST_F(ServiceWorkerProviderHostTest, ActiveIsNotController) {
true /* is_parent_frame_secure */);
remote_endpoints_.emplace_back();
remote_endpoints_.back().BindWithProviderHostInfo(&info);
auto container = base::MakeUnique<MockServiceWorkerContainer>(
std::move(*remote_endpoints_.back().client_request()));

// Create an installing version and then start the navigation.
// Associate it with an installing registration then start the navigation.
scoped_refptr<ServiceWorkerVersion> version = new ServiceWorkerVersion(
registration1_.get(), GURL("https://www.example.com/sw.js"),
1 /* version_id */, helper_->context()->AsWeakPtr());
registration1_->SetInstallingVersion(version);
SimulateServiceWorkerControlleeRequestHandler(host.get(),
registration1_.get());

// Promote the worker to active while navigation is still happening.
registration1_->SetActiveVersion(version);

// Finish the navigation.
host->SetDocumentUrl(GURL("https://www.example.com/page"));
host->CompleteNavigationInitialized(
helper_->mock_render_process_id(), std::move(info),
helper_->GetDispatcherHostForProcess(helper_->mock_render_process_id())
->AsWeakPtr());

host->AssociateRegistration(registration1_.get(),
false /* notify_controllerchange */);
// Promote the worker to active while navigation is still happening.
registration1_->SetActiveVersion(version);
base::RunLoop().RunUntilIdle();

// The page should not be controlled since there was no active version at the
// time navigation started. Furthermore, no SetController IPC should have been
// sent.
EXPECT_TRUE(host->active_version());
EXPECT_FALSE(host->controller());
EXPECT_FALSE(container->was_set_controller_called());
EXPECT_EQ(nullptr, helper_->ipc_sink()->GetFirstMessageMatching(
ServiceWorkerMsg_SetControllerServiceWorker::ID));
}

} // namespace content
62 changes: 46 additions & 16 deletions content/child/service_worker/service_worker_dispatcher.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ void ServiceWorkerDispatcher::OnMessageReceived(const IPC::Message& msg) {
OnSetVersionAttributes)
IPC_MESSAGE_HANDLER(ServiceWorkerMsg_UpdateFound,
OnUpdateFound)
IPC_MESSAGE_HANDLER(ServiceWorkerMsg_SetControllerServiceWorker,
OnSetControllerServiceWorker)
IPC_MESSAGE_HANDLER(ServiceWorkerMsg_MessageToDocument,
OnPostMessage)
IPC_MESSAGE_HANDLER(ServiceWorkerMsg_CountFeature, OnCountFeature)
Expand Down Expand Up @@ -184,18 +186,9 @@ void ServiceWorkerDispatcher::AddProviderClient(
}

void ServiceWorkerDispatcher::RemoveProviderClient(int provider_id) {
auto iter = provider_clients_.find(provider_id);
// This could be possibly called multiple times to ensure termination.
if (iter != provider_clients_.end())
provider_clients_.erase(iter);
}

blink::WebServiceWorkerProviderClient*
ServiceWorkerDispatcher::GetProviderClient(int provider_id) {
auto iter = provider_clients_.find(provider_id);
if (iter != provider_clients_.end())
return iter->second;
return nullptr;
if (base::ContainsKey(provider_clients_, provider_id))
provider_clients_.erase(provider_id);
}

ServiceWorkerDispatcher*
Expand Down Expand Up @@ -224,11 +217,6 @@ ServiceWorkerDispatcher* ServiceWorkerDispatcher::GetThreadSpecificInstance() {
g_dispatcher_tls.Pointer()->Get());
}

std::unique_ptr<ServiceWorkerHandleReference> ServiceWorkerDispatcher::Adopt(
const ServiceWorkerObjectInfo& info) {
return ServiceWorkerHandleReference::Adopt(info, thread_safe_sender_.get());
}

void ServiceWorkerDispatcher::WillStopCurrentWorkerThread() {
delete this;
}
Expand Down Expand Up @@ -522,6 +510,43 @@ void ServiceWorkerDispatcher::OnUpdateFound(
found->second->OnUpdateFound();
}

void ServiceWorkerDispatcher::OnSetControllerServiceWorker(
const ServiceWorkerMsg_SetControllerServiceWorker_Params& params) {
TRACE_EVENT2(
"ServiceWorker", "ServiceWorkerDispatcher::OnSetControllerServiceWorker",
"Thread ID", params.thread_id, "Provider ID", params.provider_id);

// Adopt the reference sent from the browser process and pass it to the
// provider context if it exists.
std::unique_ptr<ServiceWorkerHandleReference> handle_ref =
Adopt(params.object_info);
ProviderContextMap::iterator provider =
provider_contexts_.find(params.provider_id);
if (provider != provider_contexts_.end()) {
provider->second->SetController(std::move(handle_ref),
params.used_features);
}

ProviderClientMap::iterator found =
provider_clients_.find(params.provider_id);
if (found != provider_clients_.end()) {
// Sync the controllee's use counter with the service worker's one.
for (uint32_t feature : params.used_features)
found->second->CountFeature(feature);

// Get the existing worker object or create a new one with a new reference
// to populate the .controller field.
scoped_refptr<WebServiceWorkerImpl> worker =
GetOrCreateServiceWorker(ServiceWorkerHandleReference::Create(
params.object_info, thread_safe_sender_.get()));
found->second->SetController(WebServiceWorkerImpl::CreateHandle(worker),
params.should_notify_controllerchange);
// You must not access |found| after setController() because it may fire the
// controllerchange event that may remove the provider client, for example,
// by detaching an iframe.
}
}

void ServiceWorkerDispatcher::OnPostMessage(
const ServiceWorkerMsg_MessageToDocument_Params& params) {
// Make sure we're on the main document thread. (That must be the only
Expand Down Expand Up @@ -586,4 +611,9 @@ void ServiceWorkerDispatcher::RemoveServiceWorkerRegistration(
registrations_.erase(registration_handle_id);
}

std::unique_ptr<ServiceWorkerHandleReference> ServiceWorkerDispatcher::Adopt(
const ServiceWorkerObjectInfo& info) {
return ServiceWorkerHandleReference::Adopt(info, thread_safe_sender_.get());
}

} // namespace content
12 changes: 5 additions & 7 deletions content/child/service_worker/service_worker_dispatcher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
blink::WebServiceWorkerProviderClient* client);
void RemoveProviderClient(int provider_id);

blink::WebServiceWorkerProviderClient* GetProviderClient(int provider_id);

// Returns the existing service worker or a newly created one with the given
// handle reference. Returns nullptr if the given reference is invalid.
scoped_refptr<WebServiceWorkerImpl> GetOrCreateServiceWorker(
Expand Down Expand Up @@ -141,11 +139,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
// instance if thread-local instance doesn't exist.
static ServiceWorkerDispatcher* GetThreadSpecificInstance();

// Assumes that the given object information retains an interprocess handle
// reference passed from the browser process, and adopts it.
std::unique_ptr<ServiceWorkerHandleReference> Adopt(
const ServiceWorkerObjectInfo& info);

base::SingleThreadTaskRunner* main_thread_task_runner() {
return main_thread_task_runner_.get();
}
Expand Down Expand Up @@ -234,6 +227,11 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
void RemoveServiceWorkerRegistration(
int registration_handle_id);

// Assumes that the given object information retains an interprocess handle
// reference passed from the browser process, and adopts it.
std::unique_ptr<ServiceWorkerHandleReference> Adopt(
const ServiceWorkerObjectInfo& info);

UpdateCallbackMap pending_update_callbacks_;
UnregistrationCallbackMap pending_unregistration_callbacks_;
EnableNavigationPreloadCallbackMap enable_navigation_preload_callbacks_;
Expand Down

0 comments on commit 75baffd

Please sign in to comment.