Dispatch IndexedDB IPC messages to worker threads

content/renderer/indexed_db_message_filter.h

Index: content/renderer/indexed_db_message_filter.h
diff --git a/content/renderer/indexed_db_message_filter.h b/content/renderer/indexed_db_message_filter.h
new file mode 100644
index 0000000000000000000000000000000000000000..fbac654a7c5a7827f9551bd276b88500de9c0016
--- /dev/null
+++ b/content/renderer/indexed_db_message_filter.h
@@ -0,0 +1,118 @@
+// Copyright (c) 2011 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_RENDERER_INDEXED_DB_MESSAGE_FILTER_H_
+#define CONTENT_RENDERER_INDEXED_DB_MESSAGE_FILTER_H_
+#pragma once
+
+#include <map>
+#include <set>
+
+#include "base/nullable_string16.h"
+#include "base/threading/thread_local.h"
+#include "content/renderer/webcore_worker_tracker.h"
+#include "ipc/ipc_channel_proxy.h"
+
+class IndexedDBDispatcher;
+class IndexedDBKey;
+
+namespace content {
+class SerializedScriptValue;
+}
+
+namespace WebKit {
+class WebIDBCallbacks;
+class WebIDBDatabaseCallbacks;
+class WebIDBTransactionCallbacks;
+class WebWorkerRunLoop;
+}
+
+class IndexedDBMessageFilter : public IPC::ChannelProxy::MessageFilter,
+                               public WebCoreWorkerTracker {
+ public:
+  IndexedDBMessageFilter();
+  virtual ~IndexedDBMessageFilter();
+
+  // IPC::Channel::Listener implementation.
+  virtual bool OnMessageReceived(const IPC::Message& msg);
+
+  // This returns the dispatcher associated with the current WebWorkerRunLoop,
+  // if being run from a worker.
+  IndexedDBDispatcher* thread_specific_idb_dispatcher();
+
+  virtual void DidStartWorkerRunLoop(
+      const WebKit::WebWorkerRunLoop& loop) OVERRIDE;
+  virtual void DidStopWorkerRunLoop(
+      const WebKit::WebWorkerRunLoop& loop) OVERRIDE;

[hans, 2011/11/30 15:05:40]

I was confused by the "Did" in these names... but I suppose the point is that
they get called by something when "loop" has been started or stopped? Maybe add
a comment?

[dgrogan, 2011/11/30 23:25:18]

Comment added.

+  virtual WebKit::WebWorkerRunLoop* current() OVERRIDE;
+
+  int GetUniqueID();
+  void RemoveForType(int32 id, WebKit::WebIDBCallbacks* callbacks);
+  void RemoveForType(int32 id, WebKit::WebIDBTransactionCallbacks* callbacks);
+  void RemoveForType(int32 id, WebKit::WebIDBDatabaseCallbacks* callbacks);
+  void NotifyAddWithIDForType(int32 id,
+                              WebKit::WebIDBTransactionCallbacks* callbacks);
+  void NotifyAddWithIDForType(int32 id,
+                              WebKit::WebIDBDatabaseCallbacks* callbacks);

[michaeln, 2011/11/30 21:48:14]

Feels tedious to maintain maps per callback/object type. Did you consider
assigning a workerId for each unique worker, and having a single map<id,
webworkerloop> and adding that id to the IPC messages. There's be need for per
call map maintance, and message specific map lookup.

[dgrogan, 2011/11/30 23:25:18]

Agreed.
Yeah, that's what I wanted to do initially.  The issue was that the ids for
WebIDBTransactionCallbacks and WebIDBDatabaseCallbacks are generated by the
browser process.  I didn't want to mess with the browser side too much, though
it's arguable that transferring control over assigning ids to the renderer
process would be less invasive than these maps.

+
+ private:
+  void OnSuccessNull(const IPC::Message& msg, int32 response_id);
+  void OnSuccessIDBDatabase(const IPC::Message& msg, int32 response_id,
+                            int32 object_id);
+  void OnSuccessIndexedDBKey(const IPC::Message& msg, int32 response_id,
+                             const IndexedDBKey& key);
+  void OnSuccessIDBTransaction(const IPC::Message& msg, int32 response_id,
+                               int32 object_id);
+  void OnSuccessOpenCursor(const IPC::Message& msg, int32 response_id,
+      int32 object_id, const IndexedDBKey& key, const IndexedDBKey& primary_key,
+      const content::SerializedScriptValue& value);
+  void OnSuccessCursorContinue(const IPC::Message& msg, int32 response_id,
+                               int32 cursor_id,
+                               const IndexedDBKey& key,
+                               const IndexedDBKey& primary_key,
+                               const content::SerializedScriptValue& value);
+  void OnSuccessStringList(const IPC::Message& msg, int32 response_id,
+      const std::vector<string16>& value);
+  void OnSuccessSerializedScriptValue(const IPC::Message& msg,
+      int32 response_id, const content::SerializedScriptValue& value);
+  void OnError(const IPC::Message& msg, int32 response_id, int code,
+               const string16& message);
+  void OnBlocked(const IPC::Message& msg, int32 response_id);
+  void OnAbort(const IPC::Message& msg, int32 transaction_id);
+  void OnComplete(const IPC::Message& msg, int32 transaction_id);
+  void OnVersionChange(const IPC::Message& msg, int32 database_id,
+                       const string16& newVersion);
+
+  void ForwardToMainThread(const IPC::Message& msg);
+  scoped_refptr<base::MessageLoopProxy> main_thread_loop_proxy_;
+  scoped_refptr<IndexedDBDispatcher> main_thread_dispatcher_;
+
+  typedef std::map<WebKit::WebWorkerRunLoop*,
+          scoped_refptr<IndexedDBDispatcher> > LoopToDispatcherMap;
+  LoopToDispatcherMap dispatchers_;
+  base::Lock dispatchers_lock_;
+
+  typedef std::set<WebKit::WebWorkerRunLoop> LoopSet;
+  LoopSet run_loops_;

[michaeln, 2011/11/30 21:48:14]

I'd vote to factor this collection and the TLS value out of this class and into
something that's not IDB specific. I think that could help simplify this class
and also make it easier for another consumer to use this WebWorkerRunLoop class.

[dgrogan, 2011/11/30 23:25:18]

The idea behind the WebCoreWorkerTracker interface was to allow future reuse
without burdening this change too much.  I suppose the burden would be that this
class would still have to be notified when worker runloops stop so that it could
manage the IndexedDBDispatcher objects' memory.

+  base::Lock run_loops_lock_;
+  virtual bool Contains(const WebKit::WebWorkerRunLoop& run_loop);

[michaeln, 2011/11/30 21:48:14]

why virtual?

[dgrogan, 2011/11/30 23:25:18]

Mistake.  Removed.

+
+  int unique_id_ = 0;
+  base::Lock id_lock_;
+
+  typedef std::map<int, WebKit::WebWorkerRunLoop*> IDToRunLoop;

[hans, 2011/11/30 15:05:40]

maybe IDToRunLoopMap?

[dgrogan, 2011/11/30 21:34:35]

Done.

+  IDToRunLoop id_to_run_loop_;
+  IDToRunLoop database_id_to_run_loop_;
+  IDToRunLoop transaction_id_to_run_loop_;
+  void RemoveFromMap(int32 id, IDToRunLoop*);

[michaeln, 2011/11/30 21:48:14]

please put typedefs, methods, members in order

[dgrogan, 2011/11/30 23:25:18]

will do

+  base::Lock map_lock_;
+
+  void ForwardToThread(const IPC::Message& msg, int32 id,
+                       const IDToRunLoop& map);
+  base::ThreadLocalPointer<WebKit::WebWorkerRunLoop> tls_worker_run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(IndexedDBMessageFilter);
+};
+
+#endif  // CONTENT_RENDERER_INDEXED_DB_DISPATCHER_H_