Move sync notifier contruction out of syncer thread.

Move sync notifier contruction out of syncer thread.

Add thread safety checks to ensure that all the methods are called on
the same thread.

BUG=
TEST=


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

[nilesh, 2011-04-04 18:11:07]

Added a worker thread on P2PNotifier to satisfy thread safety constraints by
TalkMediatorImpl and MediatorThreadImpl

[nilesh, 2011-04-05 20:52:36]

Thanks for the feedback. I have relaxed the thread safety checks in
TalkMediatorImpl and MediatorThreadImpl. Please have a look.

[akalin, 2011-04-05 22:02:46]

Few comments

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/engine/s...
File chrome/browser/sync/engine/syncapi_unittest.cc (right):

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/engine/s...
chrome/browser/sync/engine/syncapi_unittest.cc:656:
sync_notifier_mock_.reset(new StrictMock<SyncNotifierMock>());
need extra space in beginning

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/notifier...
File chrome/browser/sync/notifier/non_blocking_invalidation_notifier.cc (right):

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/notifier...
chrome/browser/sync/notifier/non_blocking_invalidation_notifier.cc:45: if
(!method_message_loop_) {
Same thing as TalkMediatorImpl

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/notifier...
File chrome/browser/sync/notifier/p2p_notifier.cc (right):

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/notifier...
chrome/browser/sync/notifier/p2p_notifier.cc:42: // Can be the first method to
be called. Assign method_message_loop_ if
See comment in TalkMediatorImpl

http://codereview.chromium.org/6794005/diff/3030/jingle/notifier/listener/med...
File jingle/notifier/listener/mediator_thread_impl.cc (right):

http://codereview.chromium.org/6794005/diff/3030/jingle/notifier/listener/med...
jingle/notifier/listener/mediator_thread_impl.cc:43: // Can be the first method
to be called. Assign method_message_loop_ if
See TalkMediatorImpl comment.

http://codereview.chromium.org/6794005/diff/3030/jingle/notifier/listener/tal...
File jingle/notifier/listener/talk_mediator_impl.cc (right):

http://codereview.chromium.org/6794005/diff/3030/jingle/notifier/listener/tal...
jingle/notifier/listener/talk_mediator_impl.cc:75: // Is the first method to be
called. Assign method_message_loop_.
The requirement that SetAuthToken() be called first is kind of brittle.  I'd
prefer if this code is moved to a helper function and have all the methods
intended to be called on the method_message_loop_ call that function instead.

[nilesh, 2011-04-05 23:36:17]

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/engine/s...
File chrome/browser/sync/engine/syncapi_unittest.cc (right):

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/engine/s...
chrome/browser/sync/engine/syncapi_unittest.cc:656:
sync_notifier_mock_.reset(new StrictMock<SyncNotifierMock>());
On 2011/04/05 22:02:46, akalin wrote:
> need extra space in beginning

Done.

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/notifier...
File chrome/browser/sync/notifier/non_blocking_invalidation_notifier.cc (right):

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/notifier...
chrome/browser/sync/notifier/non_blocking_invalidation_notifier.cc:45: if
(!method_message_loop_) {
On 2011/04/05 22:02:46, akalin wrote:
> Same thing as TalkMediatorImpl

Done.

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/notifier...
File chrome/browser/sync/notifier/p2p_notifier.cc (right):

http://codereview.chromium.org/6794005/diff/3030/chrome/browser/sync/notifier...
chrome/browser/sync/notifier/p2p_notifier.cc:42: // Can be the first method to
be called. Assign method_message_loop_ if
On 2011/04/05 22:02:46, akalin wrote:
> See comment in TalkMediatorImpl

Done.

http://codereview.chromium.org/6794005/diff/3030/jingle/notifier/listener/med...
File jingle/notifier/listener/mediator_thread_impl.cc (right):

http://codereview.chromium.org/6794005/diff/3030/jingle/notifier/listener/med...
jingle/notifier/listener/mediator_thread_impl.cc:43: // Can be the first method
to be called. Assign method_message_loop_ if
On 2011/04/05 22:02:46, akalin wrote:
> See TalkMediatorImpl comment.

Done.

http://codereview.chromium.org/6794005/diff/3030/jingle/notifier/listener/tal...
File jingle/notifier/listener/talk_mediator_impl.cc (right):

http://codereview.chromium.org/6794005/diff/3030/jingle/notifier/listener/tal...
jingle/notifier/listener/talk_mediator_impl.cc:75: // Is the first method to be
called. Assign method_message_loop_.
On 2011/04/05 22:02:46, akalin wrote:
> The requirement that SetAuthToken() be called first is kind of brittle.  I'd
> prefer if this code is moved to a helper function and have all the methods
> intended to be called on the method_message_loop_ call that function instead.

Done.

[akalin, 2011-04-06 20:35:26]

LGTM

http://codereview.chromium.org/6794005/diff/2004/chrome/browser/sync/notifier...
File chrome/browser/sync/notifier/p2p_notifier.cc (right):

http://codereview.chromium.org/6794005/diff/2004/chrome/browser/sync/notifier...
chrome/browser/sync/notifier/p2p_notifier.cc:4: //
Move this note near the code in RemoveObserver.

Add a TODO for me to fix this.

http://codereview.chromium.org/6794005/diff/2004/jingle/notifier/listener/tal...
File jingle/notifier/listener/talk_mediator_impl.h (right):

http://codereview.chromium.org/6794005/diff/2004/jingle/notifier/listener/tal...
jingle/notifier/listener/talk_mediator_impl.h:44: // All the method below should
be called on the same thread. It may or may
method -> methods

[nilesh, 2011-04-06 21:58:15]

Thanks for the review.

http://codereview.chromium.org/6794005/diff/2004/chrome/browser/sync/notifier...
File chrome/browser/sync/notifier/p2p_notifier.cc (right):

http://codereview.chromium.org/6794005/diff/2004/chrome/browser/sync/notifier...
chrome/browser/sync/notifier/p2p_notifier.cc:4: //
On 2011/04/06 20:35:26, akalin wrote:
> Move this note near the code in RemoveObserver.
> 
> Add a TODO for me to fix this.

Done.

http://codereview.chromium.org/6794005/diff/2004/jingle/notifier/listener/tal...
File jingle/notifier/listener/talk_mediator_impl.h (right):

http://codereview.chromium.org/6794005/diff/2004/jingle/notifier/listener/tal...
jingle/notifier/listener/talk_mediator_impl.h:44: // All the method below should
be called on the same thread. It may or may
On 2011/04/06 20:35:26, akalin wrote:
> method -> methods

Done.

[commit-bot: I haz the power, 2011-04-07 00:08:19]

Change committed as 80724