Make JingleThreadMessageLoop usable without JingleThread.

remoting/jingle_glue/jingle_thread.h

Index: remoting/jingle_glue/jingle_thread.h
diff --git a/remoting/jingle_glue/jingle_thread.h b/remoting/jingle_glue/jingle_thread.h
index a7201e1bd0ceb8143cd747e3758c37b5d376dc03..11807b2253da305014b19d6d7f934dbb91590894 100644
--- a/remoting/jingle_glue/jingle_thread.h
+++ b/remoting/jingle_glue/jingle_thread.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -31,6 +31,22 @@ class TaskPump : public talk_base::MessageHandler,
   virtual void OnMessage(talk_base::Message* pmsg);
 };
 
+class JingleThreadMessageLoop : public MessageLoop {

[Wez, 2011/07/01 20:33:00]

Isn't this really a Jingle-aware MessageLoop, so no need for Thread in the name?

[Sergey Ulanov, 2011/07/01 21:14:28]

It's a MessageLoop that runs tasks on a libjingle thread. JingleMessageLoop - is
very confusing. For example it may be interpreted as a MessageLoop that can run
libjingle tasks/messages.

[Wez, 2011/07/01 21:49:28]

OK, understood.

+ public:
+  JingleThreadMessageLoop(talk_base::Thread* thread);
+  virtual ~JingleThreadMessageLoop();
+
+  void Initialize();
+
+ private:
+  // AutoRunState sets |state_| for this message loop. It needs to be
+  // created here because we never call Run() or RunAllPending() for

[Wez, 2011/07/01 20:33:00]

nit: It's not being created here, this is just the definition of the member that
holds it.

AutoRunState is actually for use when nesting MessageLoop::Run() calls, to
ensure that only the top-most Run() is exited by Quit() calls made within it. 
It also seems to be an un-documented detail within MessageLoop, so I'm dubious
about using it, especially in this context.

Would it make more sense to call talk_base::Thread::Run() *through*
JingleThreadMessageLoop::Run()?  The latter Run call would then create the
JingleMessagePump, create the AutoRunState on the stack, and call Run() on the
pump with itself as the delegate; the pump would then call
talk_base::Thread::Run().

[Sergey Ulanov, 2011/07/01 21:14:28]

Moved the comment.
Yes, it's a bit hacky, but MessageLoop was never intended to be used in a ways
we use it here. We will be able to get rid of this code when we switch to using
chromium-based IO in the host.
Yes, we have something like this in jingle/glue/thread_wrapper.[h|cc]. This
class accomplishes the opposite task: it runs MessageLoop on a jingle thread.
The reason we need it is because we use libjingle-based IO (on host and in
tests) which works only on a thread that runs talk_base::Thread::Run(). If try
to use libjingle IO code on a regular chromium code it will not work because
there is no way it can get async IO notifications.

[Wez, 2011/07/01 21:49:28]

Yes, but what I'm suggesting is that since we are in control of the
JingleThread::Run(), we can have it create the MessageLoop & Run() it, then have
that Run() the MessagePump, and finally have the MessagePump call
talk_base::Thread::Run()?  That way jingle has the right kind of thread to run
on.

[Sergey Ulanov, 2011/07/01 22:40:54]

Done.

+  // the thread.
+  scoped_ptr<AutoRunState> jingle_message_loop_state_;
+
+  DISALLOW_COPY_AND_ASSIGN(JingleThreadMessageLoop);
+};
+
 // TODO(sergeyu): This class should be changed to inherit from Chromiums
 // base::Thread instead of libjingle's thread.
 class JingleThread : public talk_base::Thread,
@@ -56,9 +72,6 @@ class JingleThread : public talk_base::Thread,
   TaskPump* task_pump();
 
  private:
-  class JingleMessageLoop;
-  class JingleMessagePump;
-
   virtual void OnMessage(talk_base::Message* msg);
 
   TaskPump* task_pump_;