Run a Chrome message loop on the mediator thread, allowing certificates to be...

Run a Chrome message loop on the mediator thread, allowing certificates to be verified.

BUG=none
TEST= 1.  Enable bookmark syncing on a mac or linux build.
2.  Run chrome with --log-level=0.
3.  Options menu -> Sync my Bookmarks.  Log in.
4.  Check that "XmppSocketAdapter::OnConnectEvent - STATE_TLS_OPEN" appears in the output.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30259

[chron_chromium.org, 2009-10-27 19:00:14]

Will message loop shutdown compete with anything, like the current talk loo=
p?



On Tue, Oct 27, 2009 at 11:56 AM,  <zork@chromium.org> wrote:
> Reviewers: chron_chromium.org,
>
> Description:
> Run a Chrome message loop on the mediator thread, allowing certificates t=
o
> be
> verified.
>
> BUG=3Dnone
> TEST=3D 1. =A0Enable bookmark syncing on a mac or linux build.
> 2. =A0Run chrome with --log-level=3D0.
> 3. =A0Options menu -> Sync my Bookmarks. =A0Log in.
> 4. =A0Check that "XmppSocketAdapter::OnConnectEvent - STATE_TLS_OPEN" app=
ears
> in
> the output.
>
>
> Please review this at http://codereview.chromium.org/340007
>
> SVN Base: svn://chrome-svn/chrome/trunk/src/
>
> Affected files:
> =A0M =A0 =A0 chrome/browser/sync/notifier/listener/mediator_thread_impl.c=
c
>
>
> Index: chrome/browser/sync/notifier/listener/mediator_thread_impl.cc
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- chrome/browser/sync/notifier/listener/mediator_thread_impl.cc
> (revision 30214)
> +++ chrome/browser/sync/notifier/listener/mediator_thread_impl.cc
> (working copy)
> @@ -5,6 +5,7 @@
> =A0#include "chrome/browser/sync/notifier/listener/mediator_thread_impl.h=
"
>
> =A0#include "base/logging.h"
> +#include "base/message_loop.h"
> =A0#include "base/platform_thread.h"
> =A0#include "chrome/browser/sync/engine/net/gaia_authenticator.h"
> =A0#include "chrome/browser/sync/notifier/base/async_dns_lookup.h"
> @@ -41,6 +42,8 @@
> =A0 PlatformThread::SetName("SyncEngine_MediatorThread");
> =A0 // For win32, this sets up the win32socketserver. Note that it needs =
to
> =A0 // dispatch windows messages since that is what the win32 socket serv=
er
> uses.
> +
> + =A0MessageLoop message_loop;
> =A0#if defined(OS_WIN)
> =A0 scoped_ptr<talk_base::SocketServer> socket_server(
> =A0 =A0 =A0 new talk_base::Win32SocketServer(this));
> @@ -57,10 +60,14 @@
> =A0 =A0 if (IsStopping()) {
> =A0 =A0 =A0 break;
> =A0 =A0 }
> + =A0 =A0MessageLoop::current()->RunAllPending();
> =A0 }
> =A0#endif
>
> - =A0ProcessMessages(talk_base::kForever);
> + =A0do {
> + =A0 =A0ProcessMessages(100);
> + =A0 =A0MessageLoop::current()->RunAllPending();
> + =A0} while (!IsStopping());
>
> =A0#if defined(OS_WIN)
> =A0 set_socketserver(old_socket_server);
>
>
>



--=20
As seen on TV

[Zachary Kuznia, 2009-10-27 21:35:17]

It will not compete.  Since message_loop is a local variable, it gets cleaned up
at the end of this function.  libjingle's message queue is cleaned up later.

[tim (not reviewing), 2009-10-27 22:01:07]

On 2009/10/27 21:35:17, zork wrote:
> It will not compete.  Since message_loop is a local variable, it gets cleaned
up
> at the end of this function.  libjingle's message queue is cleaned up later.

This works? What changed? I remember you mentioned there were challenges in
doing this when I suggested we try it out because the thread implementations
were "incompatible".

In either case, it seems really bad to run for a hard coded amount of time,
rather than use Run() and calling Quit() from someplace.  How do we know the
task was run in the window of time you specify? Am I missing something?

[chron, 2009-10-28 21:52:18]

LGTM

On Tue, Oct 27, 2009 at 2:35 PM,  <zork@chromium.org> wrote:
> It will not compete. =A0Since message_loop is a local variable, it gets
> cleaned up
> at the end of this function. =A0libjingle's message queue is cleaned up l=
ater.
>
> http://codereview.chromium.org/340007
>



--=20
As seen on TV