Implement the ServiceWorker "activate" event during activation

content/browser/service_worker/embedded_worker_registry.cc

 // Copyright 2013 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 "content/browser/service_worker/embedded_worker_registry.h"
 
 #include "base/stl_util.h"
 #include "content/browser/service_worker/embedded_worker_instance.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/common/service_worker/embedded_worker_messages.h"
@@ skipping 51 matching lines @@
   WorkerInstanceMap::iterator found = worker_map_.find(embedded_worker_id);
   if (found == worker_map_.end()) {
     LOG(ERROR) << "Worker " << embedded_worker_id << " not registered";
     return;
   }
   DCHECK_EQ(found->second->process_id(), process_id);
   worker_process_map_[process_id].erase(embedded_worker_id);
   found->second->OnStopped();
 }
 
 void EmbeddedWorkerRegistry::OnSendMessageToBrowser(

[jschuh, 2014/03/28 21:27:29]

This seems badly named. Where exactly are we running at this point?

[dominicc (has gone to gerrit), 2014/03/31 03:59:33]

This method is named OnSendMessageToBrowser because it's the receiving side of a
method on the ServiceWorker side called SendMessageToBrowser. I agree it is
mildly confusing since it turns around and sends a message (but not to browser)
therefore one could misinterpret this as "OnBeforeSend..." But it is not that.

[kinuko, 2014/03/31 04:31:03]

I agree it could be confusing, but so far we don't seem to have a better
consensus for method naming for receiving posted / sent messages other than this
style (e.g. I could find some precedence of 'OnPostMessage' etc for receiving
posted messages)

Maybe just removing 'Send' works in our case?

[jschuh, 2014/03/31 13:36:15]

Yeah, I'm not worried now that I understand what this does. But it's very
confusing to have a message handler with this name under content/browser but
actually running in a sandboxed process. Maybe the name is fine, but we need
some sort of comment to explain what's going on here

     int embedded_worker_id, int request_id, const IPC::Message& message) {
   WorkerInstanceMap::iterator found = worker_map_.find(embedded_worker_id);
   if (found == worker_map_.end()) {
     LOG(ERROR) << "Worker " << embedded_worker_id << " not registered";
     return;
   }
   // Perform security check to filter out any unexpected (and non-test)
   // messages. This must list up all message types that can go through here.
-  if (message.type() == ServiceWorkerHostMsg_InstallEventFinished::ID ||
+  if (message.type() == ServiceWorkerHostMsg_ActivateEventFinished::ID ||

[jschuh, 2014/03/28 21:27:29]

Why are we filtering here? Shouldn't found->second->OnMessageReceived() have an
explicit message map?

[dominicc (has gone to gerrit), 2014/03/31 03:59:33]

This class acts as a proxy routing messages to a worker; the point of checking
here is to limit the kinds of messages sent via this  payload in the event the
renderer is compromised. The kinds of methods that should go via this route may
be fewer than all the kinds of messages the worker responds to.

This is based on advice from Tom Sepez here:
<https://codereview.chromium.org/140893002#msg7>.

[jschuh, 2014/03/31 13:36:15]

Okay, I guess @tsepez and I disagree on the value f this, but it certainly
doesn't hurt anything.

+      message.type() == ServiceWorkerHostMsg_InstallEventFinished::ID ||
       message.type() == ServiceWorkerHostMsg_FetchEventFinished::ID ||
       IPC_MESSAGE_CLASS(message) == TestMsgStart) {
     found->second->OnMessageReceived(request_id, message);
     return;
   }
   NOTREACHED() << "Got unexpected message: " << message.type();
 }
 
 void EmbeddedWorkerRegistry::AddChildProcessSender(
     int process_id, IPC::Sender* sender) {
@@ skipping 41 matching lines @@
 }
 
 void EmbeddedWorkerRegistry::RemoveWorker(int process_id,
                                           int embedded_worker_id) {
   DCHECK(ContainsKey(worker_map_, embedded_worker_id));
   worker_map_.erase(embedded_worker_id);
   worker_process_map_.erase(process_id);
 }
 
 }  // namespace content