Let IPC::SyncMessageFilter take advantage of thread-safe Send.

content/test/test_thread_safe_sender.h

Index: content/test/test_thread_safe_sender.h
diff --git a/content/test/test_thread_safe_sender.h b/content/test/test_thread_safe_sender.h
new file mode 100644
index 0000000000000000000000000000000000000000..8232371be3355c27cde0981cd21644fa9e34ca59
--- /dev/null
+++ b/content/test/test_thread_safe_sender.h
@@ -0,0 +1,51 @@
+// Copyright 2015 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.
+
+#ifndef CONTENT_TEST_TEST_THREAD_SAFE_SENDER_H_
+#define CONTENT_TEST_TEST_THREAD_SAFE_SENDER_H_
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "content/child/thread_safe_sender.h"
+
+namespace IPC {
+class SyncChannel;
+}
+
+namespace base {
+class SingleThreadTaskRunner;
+class Thread;
+class WaitableEvent;
+}
+
+namespace content {
+
+// This provides an implementation of ThreadSafeSender to be used from unit
+// tests. This creates its on IO thread and IPC::SyncChannel and controls their

[michaeln, 2015/08/05 01:46:15]

Is any of that actually needed in the unittests? I don't think Test impl here
sends anything anywhere. Would a simpler impl that looks more like this be
sufficient?

bool MockThreadSafeSender::Send(IPC::Message* msg) {
  delete msg;
  return true;
}

And if so, since it doesn't actually function, maybe call it 'mock' instead.

[Ken Rockot(use gerrit already), 2015/08/05 05:23:36]

No actually, the tests don't need any of this. This was part of an effort to
hide SyncMessageFilter's constructor - an SMF should be constructed by a
SyncChannel instead, since it knows whether or not it's thread-safe and can tell
the SMF as much. Constructing a SyncChannel needs all this stuff and I didn't
want tests to be duplicating all of it.

However, given that this approach turned out to be insufficient for NaCl's usage
I like your suggestion. The effort to create a mock then becomes small enough
though that it's probably not worth adding to content/test.

I updated the tests individually with a mock SMF.

+// lifetime.
+class TestThreadSafeSender : public ThreadSafeSender {
+ public:
+  static scoped_refptr<ThreadSafeSender> Create(
+      const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner);
+
+ private:
+  friend class base::RefCountedThreadSafe<ThreadSafeSender>;
+
+  TestThreadSafeSender(
+      const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner,
+      scoped_ptr<base::Thread> io_thread,
+      scoped_ptr<base::WaitableEvent> shutdown_event,
+      scoped_ptr<IPC::SyncChannel> sync_channel);
+  ~TestThreadSafeSender() override;
+
+  scoped_ptr<base::Thread> io_thread_;
+  scoped_ptr<base::WaitableEvent> shutdown_event_;
+  scoped_ptr<IPC::SyncChannel> sync_channel_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestThreadSafeSender);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_TEST_TEST_THREAD_SAFE_SENDER_H_