Limit size of typed url node.

chrome/browser/sync/glue/typed_url_model_associator.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/glue/typed_url_model_associator.h"
 
 #include <algorithm>
 #include <set>
 
 #include "base/logging.h"
 #include "base/tracked.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/history/history_backend.h"
 #include "chrome/browser/sync/api/sync_error.h"
 #include "chrome/browser/sync/engine/syncapi.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/protocol/typed_url_specifics.pb.h"
 
 namespace browser_sync {
 
+// The server backend can't handle arbitrarily large node sizes, so to keep
+// the size under control we limit the visit array.
+static const int kMaxTypedUrlVisits = 100;
+
 const char kTypedUrlTag[] = "google_chrome_typed_urls";
 
 static bool CheckVisitOrdering(const history::VisitVector& visits) {
   int64 previous_visit_time = 0;
   for (history::VisitVector::const_iterator visit = visits.begin();
        visit != visits.end(); ++visit) {
     if (visit != visits.begin() &&
         previous_visit_time >= visit->visit_time.ToInternalValue()) {
       return false;
     }
@@ skipping 499 matching lines @@
 
   new_url->set_last_visit(visits->back().visit_time);
   return different;
 }
 
 // static
 void TypedUrlModelAssociator::WriteToSyncNode(
     const history::URLRow& url,
     const history::VisitVector& visits,
     sync_api::WriteNode* node) {
+  sync_pb::TypedUrlSpecifics typed_url;
+  WriteToTypedUrlSpecifics(url, visits, &typed_url);
+  node->SetTypedUrlSpecifics(typed_url);
+}
+
+void TypedUrlModelAssociator::WriteToTypedUrlSpecifics(
+    const history::URLRow& url,
+    const history::VisitVector& visits,
+    sync_pb::TypedUrlSpecifics* typed_url) {
+
   DCHECK(!url.last_visit().is_null());
   DCHECK(!visits.empty());
   DCHECK(url.last_visit() == visits.back().visit_time);
 
-  sync_pb::TypedUrlSpecifics typed_url;
-  typed_url.set_url(url.url().spec());
-  typed_url.set_title(UTF16ToUTF8(url.title()));
-  typed_url.set_hidden(url.hidden());
+  typed_url->set_url(url.url().spec());
+  typed_url->set_title(UTF16ToUTF8(url.title()));
+  typed_url->set_hidden(url.hidden());
 
   DCHECK(CheckVisitOrdering(visits));
+
+  bool only_typed = false;
+  int skip_count = 0;
+
+  if (visits.size() > static_cast<size_t>(kMaxTypedUrlVisits)) {
+    int typed_count = 0;
+    int total = 0;
+    // Walk the passed-in visit vector and count the # typed visits.

[Rick Campbell, 2011/08/09 21:25:26]

uber-nit: ... the # of typed visits.

[Andrew T Wilson (Slow), 2011/08/10 21:11:31]

Done.

+    for (history::VisitVector::const_iterator visit = visits.begin();
+         visit != visits.end(); ++visit) {
+      PageTransition::Type transition = static_cast<PageTransition::Type>(
+          visit->transition) & PageTransition::CORE_MASK;
+      // We ignore reload visits.
+      if (transition == PageTransition::RELOAD)
+        continue;
+      total++;

[Rick Campbell, 2011/08/09 21:25:26]

nit: ++total

This is definitely deep into nit-picking territory as well, but I generally
believe that prefix should be used nearly exclusively because postfix has
special semantics -- we want the special case to stand out, rather than being
the default.

[Andrew T Wilson (Slow), 2011/08/10 21:11:31]

Done.

+      if (transition == PageTransition::TYPED)
+        typed_count++;

[Rick Campbell, 2011/08/09 21:25:26]

++typed_count

[Andrew T Wilson (Slow), 2011/08/10 21:11:31]

Done.

+    }
+
+    if (typed_count > kMaxTypedUrlVisits) {
+      only_typed = true;
+      skip_count = typed_count - kMaxTypedUrlVisits;
+    } else if (total > kMaxTypedUrlVisits) {
+      skip_count = total - kMaxTypedUrlVisits;
+    }
+  }
+
+
   for (history::VisitVector::const_iterator visit = visits.begin();
        visit != visits.end(); ++visit) {
-    typed_url.add_visits(visit->visit_time.ToInternalValue());
-    typed_url.add_visit_transitions(visit->transition);
+    PageTransition::Type transition = static_cast<PageTransition::Type>(
+        visit->transition) & PageTransition::CORE_MASK;
+    // Skip reload visits.
+    if (transition == PageTransition::RELOAD)
+      continue;
+
+    // If we only have room for typed visits, then only add typed visits.
+    if (only_typed && transition != PageTransition::TYPED)
+      continue;
+
+    if (skip_count > 0) {
+      // We have too many entries to fit, so we need to skip the oldest ones.
+      // Only skip typed URLs if there are too many typed URLs to fit.
+      if (only_typed || transition != PageTransition::TYPED) {
+        skip_count--;

[Rick Campbell, 2011/08/09 21:25:26]

--skip_count

[Andrew T Wilson (Slow), 2011/08/10 21:11:31]

Done.

+        continue;
+      }
+    }
+    typed_url->add_visits(visit->visit_time.ToInternalValue());
+    typed_url->add_visit_transitions(visit->transition);
   }
-
-  node->SetTypedUrlSpecifics(typed_url);
+  CHECK_GT(typed_url->visits_size(), 0);

[Rick Campbell, 2011/08/09 21:25:26]

Maybe DCHECK(skip_count == 0) here?

[Andrew T Wilson (Slow), 2011/08/10 21:11:31]

Done.

+  CHECK_LE(typed_url->visits_size(), kMaxTypedUrlVisits);
 }
 
 // static
 void TypedUrlModelAssociator::DiffVisits(
     const history::VisitVector& old_visits,
     const sync_pb::TypedUrlSpecifics& new_url,
     std::vector<history::VisitInfo>* new_visits,
     history::VisitVector* removed_visits) {
   size_t old_visit_count = old_visits.size();
   size_t new_visit_count = new_url.visits_size();
@@ skipping 41 matching lines @@
 bool TypedUrlModelAssociator::CryptoReadyIfNecessary() {
   // We only access the cryptographer while holding a transaction.
   sync_api::ReadTransaction trans(FROM_HERE, sync_service_->GetUserShare());
   syncable::ModelTypeSet encrypted_types;
   encrypted_types = sync_api::GetEncryptedTypes(&trans);
   return encrypted_types.count(syncable::TYPED_URLS) == 0 ||
          sync_service_->IsCryptographerReady(&trans);
 }
 
 }  // namespace browser_sync