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

chrome/browser/sync/syncable/syncable.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/syncable/syncable.h"
 
 #include <algorithm>
 #include <cstring>
 #include <functional>
 #include <iomanip>
@@ skipping 697 matching lines @@
 
   // Handle success or failure.
   if (success)
     VacuumAfterSaveChanges(snapshot);
   else
     HandleSaveChangesFailure(snapshot);
   return success;
 }
 
 void Directory::VacuumAfterSaveChanges(const SaveChangesSnapshot& snapshot) {
+  if (snapshot.dirty_metas.empty())
+    return;
+
   // Need a write transaction as we are about to permanently purge entries.
   WriteTransaction trans(FROM_HERE, VACUUM_AFTER_SAVE, this);
   ScopedKernelLock lock(this);
-  kernel_->flushed_metahandles.Push(0);  // Begin flush marker
   // Now drop everything we can out of memory.
   for (EntryKernelSet::const_iterator i = snapshot.dirty_metas.begin();
        i != snapshot.dirty_metas.end(); ++i) {
     kernel_->needle.put(META_HANDLE, i->ref(META_HANDLE));
     MetahandlesIndex::iterator found =
         kernel_->metahandles_index->find(&kernel_->needle);
     EntryKernel* entry = (found == kernel_->metahandles_index->end() ?
                           NULL : *found);
     if (entry && SafeToPurgeFromMemory(entry)) {
       // We now drop deleted metahandles that are up to date on both the client
       // and the server.
       size_t num_erased = 0;
-      int64 handle = entry->ref(META_HANDLE);
-      kernel_->flushed_metahandles.Push(handle);
       num_erased = kernel_->ids_index->erase(entry);
       DCHECK_EQ(1u, num_erased);
       num_erased = kernel_->metahandles_index->erase(entry);
       DCHECK_EQ(1u, num_erased);
 
       // Might not be in it
       num_erased = kernel_->client_tag_index->erase(entry);
       DCHECK_EQ(entry->ref(UNIQUE_CLIENT_TAG).empty(), !num_erased);
       CHECK(!kernel_->parent_id_child_index->count(entry));
       delete entry;
@@ skipping 405 matching lines @@
                                  const char* name,
                                  WriterTag writer,
                                  Directory* directory)
     : from_here_(from_here), name_(name), writer_(writer),
       directory_(directory), dirkernel_(directory->kernel_) {
   dirkernel_->observers->Notify(
       &TransactionObserver::OnTransactionStart, from_here_, writer_);
 }
 
 BaseTransaction::~BaseTransaction() {
-  dirkernel_->observers->Notify(
-      &TransactionObserver::OnTransactionEnd, from_here_, writer_);
+  if (writer_ != INVALID) {

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

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.  

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

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.  

+    dirkernel_->observers->Notify(
+        &TransactionObserver::OnTransactionEnd, from_here_, writer_);
+  }
 }
 
 ReadTransaction::ReadTransaction(const tracked_objects::Location& location,
                                  Directory* directory)
     : BaseTransaction(location, "Read", INVALID, directory) {
   Lock();
 }
 
 ReadTransaction::ReadTransaction(const tracked_objects::Location& location,
                                  const ScopedDirLookup& scoped_dir)
@@ skipping 846 matching lines @@
     if (entry->ref(NEXT_ID).IsRoot() ||
         entry->ref(NEXT_ID) != entry->ref(PREV_ID)) {
       return entry;
     }
   }
   // There were no children in the linked list.
   return NULL;
 }
 
 }  // namespace syncable