[Sync] Ensure we don't hold a transaction when we access password store.

[Sync] Ensure we don't hold a transaction when we access password store.

BUG=70658
TEST=ProfileSyncSerivcePasswordTest.EnsureNoTransactions

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

[Nicolas Zea, 2011-04-19 06:16:57]

I think this ought to do it! Looking in autofill model associator, we use the
same logic (see the comment near the end of AssociateModels). Namely that we can
guarantee consistency since any changes both from the sync model and to the
password store have to go through the DB thread, which we are currently
occupying.

[ncarter (slow), 2011-04-19 06:25:31]

Drive-by observation; no need to wait for my LGTM.

I see that you're using three parallel queues: added passwords, deleted
passwords, and updated passwords.  I'm not sure of the possible longevity of the
enqueued items; is it possible that the same item (same client tag key) could
exist in both the add and delete queues?  If so, then it's likely that the
add+delete case will be indistinguishable from the delete+readd case.  Hopefully
this isn't an issue.

[ncarter (slow), 2011-04-19 06:25:31]

Drive-by observation; no need to wait for my LGTM.

I see that you're using three parallel queues: added passwords, deleted
passwords, and updated passwords.  I'm not sure of the possible longevity of the
enqueued items; is it possible that the same item (same client tag key) could
exist in both the add and delete queues?  If so, then it's likely that the
add+delete case will be indistinguishable from the delete+readd case.  Hopefully
this isn't an issue.

[Nicolas Zea, 2011-04-19 17:49:06]

For every applychanges (where the queues get filled), there is one
commitchanges. It's possible for commitchanges to be called without apply being
called though, which is why I clear the queues in commit. And within apply, a
node can only go into one queue, so this should be safe.

On 2011/04/19 06:25:31, ncarter wrote:
> Drive-by observation; no need to wait for my LGTM.
> 
> I see that you're using three parallel queues: added passwords, deleted
> passwords, and updated passwords.  I'm not sure of the possible longevity of
the
> enqueued items; is it possible that the same item (same client tag key) could
> exist in both the add and delete queues?  If so, then it's likely that the
> add+delete case will be indistinguishable from the delete+readd case. 
Hopefully
> this isn't an issue.

[Nicolas Zea, 2011-04-19 19:12:00]

+mdm I think this could resolve the issues w.r.t. deadlocks when the password
store has to interact with the UI thread.

[Mike Mammarella, 2011-04-19 21:09:02]

On 2011/04/19 19:12:00, nzea wrote:
> +mdm I think this could resolve the issues w.r.t. deadlocks when the password
> store has to interact with the UI thread.

Nice! This looks like it will probably do the trick. I'm not quite sure why I
was thinking that it wouldn't, in email. Threading is tricky so maybe I was
seeing something that wasn't a problem... but in any event this LGTM. Thanks so
much for taking the time to work on it!