sync: don't notify from ~BaseTransaction if the writer is INVALID.

sync: don't notify from ~BaseTransaction if the writer is INVALID.

Also get rid of flushed_metahandles since it serves exactly zero purpose :)

BUG=87122
TEST=none

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

[rlarocque, 2011-11-15 01:08:56]

http://codereview.chromium.org/8565035/diff/1/chrome/browser/sync/syncable/sy...
File chrome/browser/sync/syncable/syncable.cc (right):

http://codereview.chromium.org/8565035/diff/1/chrome/browser/sync/syncable/sy...
chrome/browser/sync/syncable/syncable.cc:1161: if (writer_ != INVALID) {
What would happen if we removed this code altogether?  

The only clients of the TransactionObserver API are the sync JS classes.  I
glanced at that code and didn't see any use of "OnTransactionEnd" events, though
it's certainly possible that I missed something.  

The JS classes would still be notified of "OnTransactionWrite" events via
NotifyTransactionChangingAndEnding.  I think that's where most of the output in
the about:sync transaction log is generated.

http://codereview.chromium.org/8565035/diff/1/chrome/browser/sync/syncable/sy...
File chrome/browser/sync/syncable/syncable.h (left):

http://codereview.chromium.org/8565035/diff/1/chrome/browser/sync/syncable/sy...
chrome/browser/sync/syncable/syncable.h:1134: DebugQueue<int64, 1000>
flushed_metahandles;
I think this is the last use of DebugQueue.  Can we delete that class, too?

[rlarocque, 2011-11-15 01:23:06]

LGTM with comment.

http://codereview.chromium.org/8565035/diff/1/chrome/browser/sync/syncable/sy...
File chrome/browser/sync/syncable/syncable.cc (right):

http://codereview.chromium.org/8565035/diff/1/chrome/browser/sync/syncable/sy...
chrome/browser/sync/syncable/syncable.cc:1161: if (writer_ != INVALID) {
On 2011/11/15 01:08:56, rlarocque wrote:
> What would happen if we removed this code altogether?  
> 
> The only clients of the TransactionObserver API are the sync JS classes.  I
> glanced at that code and didn't see any use of "OnTransactionEnd" events,
though
> it's certainly possible that I missed something.  
> 
> The JS classes would still be notified of "OnTransactionWrite" events via
> NotifyTransactionChangingAndEnding.  I think that's where most of the output
in
> the about:sync transaction log is generated.  

Never mind, I see how it's used now.  We use some JavaScript magic to register
for notifications of all sorts of event types.  

I'd still like to get rid of OnTransactionStart and OnTransactionEnd
notifications, but this CL is probably not he place to do it.  I withdraw my
comment.

[tim (not reviewing), 2011-11-15 02:11:10]

Removed DebugQueue.  Will check the CQ box!

[commit-bot: I haz the power, 2011-11-15 02:11:35]

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

[commit-bot: I haz the power, 2011-11-15 02:11:42]

Presubmit check for 8565035-5001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
You might be calling functions intended only for testing from
production code. Please verify that the following usages are OK,
and email joi@chromium.org if you are seeing false positives:
  chrome/browser/sync/syncable/syncable.cc:1803
    EntryKernel* entry = GetPossibleLastChildForTest(lock, parent_id);

Presubmit checks took 2.9s to calculate.