[Sync] - add some debug info to track down issue 120709

Add some debug info to track down issue 120709.


BUG=
TEST=


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

[lipalani1, 2012-04-27 01:16:56]

Please review.

[Nicolas Zea, 2012-04-27 17:23:52]

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
File chrome/browser/sync/glue/non_frontend_data_type_controller.cc (right):

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
chrome/browser/sync/glue/non_frontend_data_type_controller.cc:199: if
(start_association_called_) {
I suspect TSAN is going to complain about this. An alternative is to use a
WaitableEvent with manual reset, signal it in StartAssociation(), and just check
IsSignaled() here. Then reset it as part of Start()

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
chrome/browser/sync/glue/non_frontend_data_type_controller.cc:224: } else {
nit: Just as a sanity check, I'd prefer an else if (type() == TypedURL) and then
else. Theoretically it shouldn't get hit, but I don't want to rule anything out.

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
File chrome/browser/sync/glue/non_frontend_data_type_controller.h (right):

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
chrome/browser/sync/glue/non_frontend_data_type_controller.h:182: bool
start_association_called_;
comment that this is for debugging bug x and todo to remove after.

[lipalani1, 2012-04-27 17:43:20]

Please review.

TSAN, I believe should not be an issue because we dont throw any errors based on
the variable.

however I do see the point in your comment and this looks elegant.

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
File chrome/browser/sync/glue/non_frontend_data_type_controller.cc (right):

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
chrome/browser/sync/glue/non_frontend_data_type_controller.cc:199: if
(start_association_called_) {
On 2012/04/27 17:23:52, nzea wrote:
> I suspect TSAN is going to complain about this. An alternative is to use a
> WaitableEvent with manual reset, signal it in StartAssociation(), and just
check
> IsSignaled() here. Then reset it as part of Start() 

Done.

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
chrome/browser/sync/glue/non_frontend_data_type_controller.cc:224: } else {
On 2012/04/27 17:23:52, nzea wrote:
> nit: Just as a sanity check, I'd prefer an else if (type() == TypedURL) and
then
> else. Theoretically it shouldn't get hit, but I don't want to rule anything
out.

Done.

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
File chrome/browser/sync/glue/non_frontend_data_type_controller.h (right):

http://codereview.chromium.org/10248006/diff/2004/chrome/browser/sync/glue/no...
chrome/browser/sync/glue/non_frontend_data_type_controller.h:182: bool
start_association_called_;
On 2012/04/27 17:23:52, nzea wrote:
> comment that this is for debugging bug x and todo to remove after.

Done.

[Nicolas Zea, 2012-04-27 17:44:37]

lgtm

[commit-bot: I haz the power, 2012-04-27 19:12:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lipalani@chromium.org/10248006/14088

[commit-bot: I haz the power, 2012-04-27 20:02:08]

Try job failure for 10248006-14088 (retry) on linux_rel for step "unit_tests".
It's a second try, previously, step "unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-04-28 01:20:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/lipalani@chromium.org/10248006/20003

[commit-bot: I haz the power, 2012-04-28 02:54:03]

Change committed as 134433