sync: fix reentrancy crash in ModelAssociationManager

sync: fix reentrancy crash in ModelAssociationManager

Don't call OnTypesLoaded unless we're IDLE, since we could be in the middle of some 
other process.  

Add a CHECK to ensure we don't re-enter ModelAssociationManager::Initialize.

BUG=134550
TEST=trigger unrecoverable error with data type while data type configuration is in progress, don't crash.


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

[Nicolas Zea, 2012-06-27 00:06:39]

no tests? :-(

LGTM otherwise

[tim (not reviewing), 2012-06-27 00:47:35]

Oh there are tests on the way!  But I wanted Lingesh to take a look while
I'm still in Kirkland.

On Tue, Jun 26, 2012 at 5:06 PM, <zea@chromium.org> wrote:

> no tests? :-(
>
> LGTM otherwise
>
>
http://codereview.chromium.**org/10687002/<http://codereview.chromium.org/106...
>

[tim (not reviewing), 2012-06-27 17:28:46]

On 2012/06/27 00:47:35, timsteele wrote:
> Oh there are tests on the way!  But I wanted Lingesh to take a look while
> I'm still in Kirkland.
> 
> On Tue, Jun 26, 2012 at 5:06 PM, <mailto:zea@chromium.org> wrote:
> 
> > no tests? :-(
> >
> > LGTM otherwise
> >
> >
>
http://codereview.chromium.**org/10687002/%3Chttp://codereview.chromium.org/1...>
> >

PTAL

[Nicolas Zea, 2012-06-27 17:52:09]

LGTM with nits

http://codereview.chromium.org/10687002/diff/11003/chrome/browser/sync/glue/f...
File chrome/browser/sync/glue/fake_data_type_controller.cc (right):

http://codereview.chromium.org/10687002/diff/11003/chrome/browser/sync/glue/f...
chrome/browser/sync/glue/fake_data_type_controller.cc:79: // error.  We should
probably find a way to use the real code an mock out
an -> and

http://codereview.chromium.org/10687002/diff/11003/chrome/browser/sync/glue/m...
File chrome/browser/sync/glue/model_association_manager_unittest.cc (right):

http://codereview.chromium.org/10687002/diff/11003/chrome/browser/sync/glue/m...
chrome/browser/sync/glue/model_association_manager_unittest.cc:220:
GetController(controllers_, syncable::BOOKMARKS)->SetDelayModelLoad();
it's kindda weird to me that  we use GetController for something we just created
and require exists. But I guess it's consistent with the other tests.

http://codereview.chromium.org/10687002/diff/11003/chrome/browser/sync/glue/m...
chrome/browser/sync/glue/model_association_manager_unittest.cc:227:
syncable::ModelTypeSet expected_types_waiting_to_load;
expected_types_waiting_to_load(syncable::BOOKMARKS);

can do that above too for |types|

[commit-bot: I haz the power, 2012-06-27 21:05:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tim@chromium.org/10687002/9003

[commit-bot: I haz the power, 2012-06-27 22:19:30]

Change committed as 144583