Add SharedWorkerServiceImpl::CheckWorkerDependency().

content/browser/shared_worker/shared_worker_service_impl_unittest.cc

Index: content/browser/shared_worker/shared_worker_service_impl_unittest.cc
diff --git a/content/browser/shared_worker/shared_worker_service_impl_unittest.cc b/content/browser/shared_worker/shared_worker_service_impl_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..af51ae7099b4a6296991b3fbcc6c2912eea65170
--- /dev/null
+++ b/content/browser/shared_worker/shared_worker_service_impl_unittest.cc
@@ -0,0 +1,426 @@
+// Copyright 2014 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 <map>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "content/browser/message_port_message_filter.h"
+#include "content/browser/shared_worker/shared_worker_message_filter.h"
+#include "content/browser/shared_worker/shared_worker_service_impl.h"
+#include "content/browser/worker_host/worker_storage_partition.h"
+#include "content/common/message_port_messages.h"
+#include "content/common/view_messages.h"
+#include "content/common/worker_messages.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "ipc/ipc_sync_message.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+namespace {
+
+class NumberGenerator : public base::RefCountedThreadSafe<NumberGenerator> {

[kinuko, 2014/03/10 04:25:43]

You can probably use base::AtomicSequenceNumber instead?

[horo, 2014/03/10 11:48:31]

Done.

+ public:
+  explicit NumberGenerator(int initial_no) : no_(initial_no) {}
+  int Next() { return ++no_; }
+
+ private:
+  friend class base::RefCountedThreadSafe<NumberGenerator>;
+  ~NumberGenerator() {}
+  int no_;
+};
+
+class MockMessagePortMessageFilter : public MessagePortMessageFilter {
+ public:
+  MockMessagePortMessageFilter(const NextRoutingIDCallback& callback,
+                               ScopedVector<IPC::Message>* message_queue)
+      : MessagePortMessageFilter(callback), message_queue_(message_queue) {}
+
+  virtual bool Send(IPC::Message* message) OVERRIDE {
+    if (message_queue_)
+      message_queue_->push_back(message);
+    return true;
+  }
+
+  void Close() {
+    message_queue_ = NULL;
+    OnChannelClosing();
+  }
+
+ private:
+  virtual ~MockMessagePortMessageFilter() {}
+  ScopedVector<IPC::Message>* message_queue_;
+};
+
+class MockSharedWorkerMessageFilter : public SharedWorkerMessageFilter {
+ public:
+  MockSharedWorkerMessageFilter(int render_process_id,
+                                ResourceContext* resource_context,
+                                const WorkerStoragePartition& partition,
+                                MessagePortMessageFilter* message_port_filter,
+                                ScopedVector<IPC::Message>* message_queue)
+      : SharedWorkerMessageFilter(render_process_id,
+                                  resource_context,
+                                  partition,
+                                  message_port_filter),
+        message_queue_(message_queue) {}
+
+  virtual bool Send(IPC::Message* message) OVERRIDE {
+    if (message_queue_)
+      message_queue_->push_back(message);
+    return true;
+  }
+
+  void Close() {
+    message_queue_ = NULL;
+    OnChannelClosing();
+  }
+
+ private:
+  virtual ~MockSharedWorkerMessageFilter() {}
+  ScopedVector<IPC::Message>* message_queue_;
+};
+
+class MockRenderer {
+ public:
+  MockRenderer(int initial_no,
+               int process_no,
+               ResourceContext* resource_context,
+               const WorkerStoragePartition& partition)
+      : num_generator_(new NumberGenerator(initial_no)),
+        message_filter_(new MockMessagePortMessageFilter(
+            base::Bind(&NumberGenerator::Next, num_generator_),
+            &queued_messages_)),
+        worker_filter_(new MockSharedWorkerMessageFilter(process_no,
+                                                         resource_context,
+                                                         partition,
+                                                         message_filter_.get(),
+                                                         &queued_messages_)) {}
+
+  ~MockRenderer() {
+    message_filter_->Close();
+    worker_filter_->Close();
+  }
+
+  bool Post(IPC::Message* message) {

[kinuko, 2014/03/10 04:25:43]

MockRenderer -> MockRendererProcessHost
Post -> OnMessageReceived

feels more natural to me?

[horo, 2014/03/10 11:48:31]

Done.

+    scoped_ptr<IPC::Message> msg(message);
+    bool message_was_ok = false;
+    const bool ret =
+        message_filter_->OnMessageReceived(*message, &message_was_ok) ||
+        worker_filter_->OnMessageReceived(*message, &message_was_ok);
+    if (message->is_sync()) {
+      CHECK(!queued_messages_.empty());
+      const IPC::Message* responce_msg = queued_messages_.back();

[kinuko, 2014/03/10 04:25:43]

responce -> response

[horo, 2014/03/10 11:48:31]

Done.

+      IPC::SyncMessage* sync_msg = static_cast<IPC::SyncMessage*>(message);
+      scoped_ptr<IPC::MessageReplyDeserializer> reply_serializer(
+          sync_msg->GetReplyDeserializer());
+      bool result = reply_serializer->SerializeOutputParameters(*responce_msg);
+      CHECK(result);
+      queued_messages_.pop_back();
+    }
+    return ret;
+  }
+
+  int QueuedMessageCount() { return queued_messages_.size(); }

[kinuko, 2014/03/10 04:25:43]

nit: make it a const method

It'd be better to return size_t if there's no reason not to do so
(EXPECT_... lines will need 'u' suffix for literals if you do so)

[horo, 2014/03/10 11:48:31]

Done.

+
+  IPC::Message* PopMessage() {

[kinuko, 2014/03/10 04:25:43]

Can this return scoped_ptr<Message> (and all Check*Msg method take
scoped_ptr<Message>)

[horo, 2014/03/10 11:48:31]

Done.

+    CHECK(queued_messages_.size());
+    IPC::Message* msg = *queued_messages_.begin();
+    queued_messages_.weak_erase(queued_messages_.begin());
+    return msg;
+  }
+
+ private:
+  ScopedVector<IPC::Message> queued_messages_;
+  scoped_refptr<NumberGenerator> num_generator_;
+  scoped_refptr<MockMessagePortMessageFilter> message_filter_;
+  scoped_refptr<MockSharedWorkerMessageFilter> worker_filter_;
+};
+
+void CreateMessagePortPair(MockRenderer* renderer,
+                           int* route_1,
+                           int* port_1,
+                           int* route_2,
+                           int* port_2) {
+  EXPECT_TRUE(renderer->Post(
+      new MessagePortHostMsg_CreateMessagePort(route_1, port_1)));
+  EXPECT_TRUE(renderer->Post(
+      new MessagePortHostMsg_CreateMessagePort(route_2, port_2)));
+  EXPECT_TRUE(
+      renderer->Post(new MessagePortHostMsg_Entangle(*port_1, *port_2)));
+  EXPECT_TRUE(
+      renderer->Post(new MessagePortHostMsg_Entangle(*port_2, *port_1)));
+}
+
+void PostCreateWorker(MockRenderer* renderer,
+                      const std::string& url,
+                      const std::string& name,
+                      unsigned long long document_id,
+                      int render_frame_route_id,
+                      int* connector_route_id) {
+  ViewHostMsg_CreateWorker_Params params;
+  params.url = GURL(url);
+  params.name = base::ASCIIToUTF16(name);
+  params.content_security_policy = base::string16();
+  params.security_policy_type = blink::WebContentSecurityPolicyTypeReport;
+  params.document_id = document_id;
+  params.render_frame_route_id = render_frame_route_id;
+  EXPECT_TRUE(
+      renderer->Post(new ViewHostMsg_CreateWorker(params, connector_route_id)));
+}
+
+void CheckWorkerProcessMsgCreateWorker(
+    IPC::Message* msg,
+    const std::string& url,
+    const std::string& name,
+    blink::WebContentSecurityPolicyType security_policy_type,
+    int route_id) {

[kinuko, 2014/03/10 04:25:43]

nit: it'd read better if these params are prefixed with expected_ (i.e.
expected_url, expected_name, ...)

[horo, 2014/03/10 11:48:31]

Done.

+  scoped_ptr<IPC::Message> message(msg);
+  EXPECT_EQ(WorkerProcessMsg_CreateWorker::ID, msg->type());
+  Tuple1<WorkerProcessMsg_CreateWorker_Params> param;
+  EXPECT_TRUE(WorkerProcessMsg_CreateWorker::Read(msg, &param));
+  EXPECT_EQ(GURL(url), param.a.url);
+  EXPECT_EQ(base::ASCIIToUTF16(name), param.a.name);
+  EXPECT_EQ(security_policy_type, param.a.security_policy_type);
+  EXPECT_EQ(route_id, param.a.route_id);
+}
+
+void CheckViewMsgWorkerCreated(IPC::Message* msg, int msg_route_id) {
+  scoped_ptr<IPC::Message> message(msg);
+  EXPECT_EQ(ViewMsg_WorkerCreated::ID, msg->type());
+  EXPECT_EQ(msg_route_id, msg->routing_id());
+}
+
+void CheckMessagePortMsgMessagesQueued(IPC::Message* msg, int msg_route_id) {
+  scoped_ptr<IPC::Message> message(msg);
+  EXPECT_EQ(MessagePortMsg_MessagesQueued::ID, msg->type());
+  EXPECT_EQ(msg_route_id, msg->routing_id());
+}
+
+void CheckWorkerMsgConnect(IPC::Message* msg,
+                           int msg_route_id,
+                           int sent_message_port_id,
+                           int routing_id) {
+  scoped_ptr<IPC::Message> message(msg);
+  EXPECT_EQ(WorkerMsg_Connect::ID, msg->type());
+  EXPECT_EQ(msg_route_id, msg->routing_id());
+  int port_id;
+  int port_route;
+  EXPECT_TRUE(WorkerMsg_Connect::Read(msg, &port_id, &port_route));
+  EXPECT_EQ(sent_message_port_id, port_id);
+  EXPECT_EQ(routing_id, port_route);
+}
+
+void CheckMessagePortMsgMessage(IPC::Message* msg,
+                                int msg_route_id,
+                                std::string expected_data) {
+  scoped_ptr<IPC::Message> message(msg);
+  EXPECT_EQ(MessagePortMsg_Message::ID, msg->type());
+  EXPECT_EQ(msg_route_id, msg->routing_id());
+  base::string16 data;
+  std::vector<int> sent_message_port_ids;
+  std::vector<int> new_routing_ids;
+  EXPECT_TRUE(MessagePortMsg_Message::Read(
+      msg, &data, &sent_message_port_ids, &new_routing_ids));
+  EXPECT_EQ(base::ASCIIToUTF16(expected_data), data);
+}
+
+void CheckViewMsgWorkerConnected(IPC::Message* msg, int msg_route_id) {
+  scoped_ptr<IPC::Message> message(msg);
+  EXPECT_EQ(ViewMsg_WorkerConnected::ID, msg->type());
+  EXPECT_EQ(msg_route_id, msg->routing_id());
+}
+
+std::vector<std::pair<std::vector<int>, std::vector<int> > >
+    g_worker_dependency_updates;

[kinuko, 2014/03/10 04:25:43]

nit: I think just having two std::vector<int> would be more readable

Also since we're in a anon namespace naming this like a global variable may feel
a bit weird.
s_ prefix is probably better (though it's still less common) or you can just
make it a static fields of SharedWorkerServiceImplTest

[horo, 2014/03/10 11:48:31]

Done.

+
+void MockUpdateWorkerDependency(const std::vector<int> added_ids,
+                                const std::vector<int> removed_ids) {
+  g_worker_dependency_updates.push_back(std::make_pair(added_ids, removed_ids));
+}
+
+}  // namespace
+
+class SharedWorkerServiceImplTest : public testing::Test {
+ protected:
+  SharedWorkerServiceImplTest()
+      : browser_thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP),
+        browser_context_(new TestBrowserContext()),
+        partition_(
+            new WorkerStoragePartition(browser_context_->GetRequestContext(),
+                                       NULL,
+                                       NULL,
+                                       NULL,
+                                       NULL,
+                                       NULL,
+                                       NULL)) {
+    SharedWorkerServiceImpl::GetInstance()
+        ->ChangeUpdateWorkerDependencyFuncForTesting(
+            MockUpdateWorkerDependency);
+  }
+
+  virtual void SetUp() {}
+  virtual void TearDown() {

[kinuko, 2014/03/10 04:25:43]

OVERRIDE

[horo, 2014/03/10 11:48:31]

Done.

+    g_worker_dependency_updates.clear();
+    SharedWorkerServiceImpl::GetInstance()->ResetForTesting();
+  }
+
+  TestBrowserThreadBundle browser_thread_bundle_;
+  scoped_ptr<TestBrowserContext> browser_context_;
+  scoped_ptr<WorkerStoragePartition> partition_;
+  DISALLOW_COPY_AND_ASSIGN(SharedWorkerServiceImplTest);
+};
+
+TEST_F(SharedWorkerServiceImplTest, SimpleTest) {

[kinuko, 2014/03/10 04:25:43]

Can you add some more comments in the test code body (e.g. 'Simulate to create a
worker.', 'Simulate to send a message to a new worker' etc)?

This test does a lot, should all of these tested in one unit test or can this be
split into multiple tests?  I wouldn't call a test 'simple' if it's doing more
than just creating a worker in a single renderer.

[horo, 2014/03/10 11:48:31]

Done.

+  scoped_ptr<MockRenderer> renderer1(new MockRenderer(
+      100, 200, browser_context_->GetResourceContext(), *partition_.get()));
+
+  int msg_port_route1_1, msg_port_id1_1;
+  int msg_port_route1_2, msg_port_id1_2;
+  CreateMessagePortPair(renderer1.get(),
+                        &msg_port_route1_1,
+                        &msg_port_id1_1,
+                        &msg_port_route1_2,
+                        &msg_port_id1_2);
+  EXPECT_EQ(101, msg_port_route1_1);
+  EXPECT_EQ(102, msg_port_route1_2);
+
+  int connector_route_id1;
+  PostCreateWorker(renderer1.get(),
+                   "http://example.com/w.js",
+                   "name",
+                   300,
+                   400,
+                   &connector_route_id1);
+  EXPECT_EQ(103, connector_route_id1);
+  EXPECT_EQ(2, renderer1->QueuedMessageCount());
+  int worker_route_id = 104;
+  CheckWorkerProcessMsgCreateWorker(renderer1->PopMessage(),
+                                    "http://example.com/w.js",
+                                    "name",
+                                    blink::WebContentSecurityPolicyTypeReport,
+                                    worker_route_id);
+  CheckViewMsgWorkerCreated(renderer1->PopMessage(), connector_route_id1);
+
+  EXPECT_TRUE(
+      renderer1->Post(new MessagePortHostMsg_QueueMessages(msg_port_id1_1)));
+  EXPECT_EQ(1, renderer1->QueuedMessageCount());
+  CheckMessagePortMsgMessagesQueued(renderer1->PopMessage(), msg_port_route1_1);
+
+  const std::vector<int> empty_ids;
+  EXPECT_TRUE(renderer1->Post(new MessagePortHostMsg_PostMessage(
+      msg_port_id1_2, base::ASCIIToUTF16("test"), empty_ids)));
+  EXPECT_EQ(0, renderer1->QueuedMessageCount());
+
+  int new_msg_port_route1 = 105;
+  EXPECT_TRUE(renderer1->Post(new ViewHostMsg_ForwardToWorker(WorkerMsg_Connect(
+      connector_route_id1, msg_port_id1_1, MSG_ROUTING_NONE))));
+  EXPECT_EQ(1, renderer1->QueuedMessageCount());
+  CheckWorkerMsgConnect(renderer1->PopMessage(),
+                        worker_route_id,
+                        msg_port_id1_1,
+                        new_msg_port_route1);
+
+  std::vector<QueuedMessage> queued_messages;
+  EXPECT_TRUE(renderer1->Post(new MessagePortHostMsg_SendQueuedMessages(
+      msg_port_id1_1, queued_messages)));
+  EXPECT_EQ(1, renderer1->QueuedMessageCount());
+  CheckMessagePortMsgMessage(
+      renderer1->PopMessage(), new_msg_port_route1, "test");
+
+  EXPECT_TRUE(
+      renderer1->Post(new WorkerHostMsg_WorkerScriptLoaded(worker_route_id)));
+  EXPECT_EQ(0, renderer1->QueuedMessageCount());
+
+  EXPECT_TRUE(renderer1->Post(
+      new WorkerHostMsg_WorkerConnected(msg_port_id1_1, worker_route_id)));
+  EXPECT_EQ(1, renderer1->QueuedMessageCount());
+  CheckViewMsgWorkerConnected(renderer1->PopMessage(), connector_route_id1);
+
+  EXPECT_TRUE(renderer1->Post(new MessagePortHostMsg_PostMessage(
+      msg_port_id1_1, base::ASCIIToUTF16("test2"), empty_ids)));
+  EXPECT_EQ(1, renderer1->QueuedMessageCount());
+  CheckMessagePortMsgMessage(
+      renderer1->PopMessage(), msg_port_route1_2, "test2");
+
+  scoped_ptr<MockRenderer> renderer2(new MockRenderer(
+      500, 600, browser_context_->GetResourceContext(), *partition_.get()));
+
+  int msg_port_route2_1, msg_port_id2_1;
+  int msg_port_route2_2, msg_port_id2_2;
+  CreateMessagePortPair(renderer2.get(),
+                        &msg_port_route2_1,
+                        &msg_port_id2_1,
+                        &msg_port_route2_2,
+                        &msg_port_id2_2);
+  EXPECT_EQ(501, msg_port_route2_1);
+  EXPECT_EQ(502, msg_port_route2_2);
+
+  EXPECT_EQ(0U, g_worker_dependency_updates.size());
+
+  int connector_route_id2;
+  PostCreateWorker(renderer2.get(),
+                   "http://example.com/w.js",
+                   "name",
+                   700,
+                   800,
+                   &connector_route_id2);
+  EXPECT_EQ(503, connector_route_id2);
+  EXPECT_EQ(1, renderer2->QueuedMessageCount());
+  CheckViewMsgWorkerCreated(renderer2->PopMessage(), connector_route_id2);
+
+  EXPECT_EQ(1U, g_worker_dependency_updates.size());
+  EXPECT_EQ(1U, g_worker_dependency_updates[0].first.size());
+  EXPECT_EQ(200, g_worker_dependency_updates[0].first[0]);
+  EXPECT_EQ(0U, g_worker_dependency_updates[0].second.size());
+
+  EXPECT_TRUE(
+      renderer2->Post(new MessagePortHostMsg_QueueMessages(msg_port_id2_1)));
+  EXPECT_EQ(1, renderer2->QueuedMessageCount());
+  CheckMessagePortMsgMessagesQueued(renderer2->PopMessage(), msg_port_route2_1);
+
+  int new_msg_port_route2 = 106;
+  EXPECT_TRUE(renderer2->Post(new ViewHostMsg_ForwardToWorker(WorkerMsg_Connect(
+      connector_route_id2, msg_port_id2_1, MSG_ROUTING_NONE))));
+  EXPECT_EQ(1, renderer1->QueuedMessageCount());
+  CheckWorkerMsgConnect(renderer1->PopMessage(),
+                        worker_route_id,
+                        msg_port_id2_1,
+                        new_msg_port_route2);
+
+  EXPECT_TRUE(renderer2->Post(new MessagePortHostMsg_SendQueuedMessages(
+      msg_port_id2_1, queued_messages)));
+  EXPECT_EQ(0, renderer2->QueuedMessageCount());
+
+  EXPECT_TRUE(renderer1->Post(
+      new WorkerHostMsg_WorkerConnected(msg_port_id2_1, worker_route_id)));
+  EXPECT_EQ(1, renderer2->QueuedMessageCount());
+  CheckViewMsgWorkerConnected(renderer2->PopMessage(), connector_route_id2);
+
+  EXPECT_TRUE(renderer2->Post(new MessagePortHostMsg_PostMessage(
+      msg_port_id2_2, base::ASCIIToUTF16("test3"), empty_ids)));
+  EXPECT_EQ(1, renderer1->QueuedMessageCount());
+  CheckMessagePortMsgMessage(
+      renderer1->PopMessage(), new_msg_port_route2, "test3");
+
+  EXPECT_TRUE(renderer1->Post(new MessagePortHostMsg_PostMessage(
+      msg_port_id2_1, base::ASCIIToUTF16("test4"), empty_ids)));
+  EXPECT_EQ(1, renderer2->QueuedMessageCount());
+  CheckMessagePortMsgMessage(
+      renderer2->PopMessage(), msg_port_route2_2, "test4");
+
+  EXPECT_EQ(1U, g_worker_dependency_updates.size());
+  renderer2.reset();
+  EXPECT_EQ(2U, g_worker_dependency_updates.size());
+  EXPECT_EQ(0U, g_worker_dependency_updates[1].first.size());
+  EXPECT_EQ(1U, g_worker_dependency_updates[1].second.size());
+  EXPECT_EQ(200, g_worker_dependency_updates[1].second[0]);
+}
+
+}  // namespace content