Add load_completed property to ThumbnailScore.

Thumbnails taken while page loading may only contain partial contents,
hence these are not desirable.

BUG=65936
TEST=with --enable-in-browser-thumbnailing --vmodule=thumbnail_generator=1, confirm that thumbnails taken while page loading were replaced with new ones properly.

Review URL: http://codereview.chromium.org/6962008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@87264 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 34 insertions, 23 deletions

diff --git a/chrome/common/thumbnail_score.cc b/chrome/common/thumbnail_score.cc
index 37c942a..055429d 100644
--- a/chrome/common/thumbnail_score.cc
+++ b/chrome/common/thumbnail_score.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -15,27 +15,25 @@ const double ThumbnailScore::kThumbnailMaximumBoringness = 0.94;
 const double ThumbnailScore::kThumbnailDegradePerHour = 0.01;
 
 // Calculates a numeric score from traits about where a snapshot was
-// taken. We store the raw components in the database because I'm sure
-// this will evolve and I don't want to break databases.
-static int GetThumbnailType(bool good_clipping, bool at_top) {
-  if (good_clipping && at_top) {
-    return 0;
-  } else if (good_clipping && !at_top) {
-    return 1;
-  } else if (!good_clipping && at_top) {
-    return 2;
-  } else if (!good_clipping && !at_top) {
-    return 3;
-  } else {
-    NOTREACHED();
-    return -1;
-  }
+// taken. The lower the better. We store the raw components in the
+// database because I'm sure this will evolve and I don't want to break
+// databases.
+static int GetThumbnailType(const ThumbnailScore& score) {
+  int type = 0;
+  if (!score.at_top)
+    type += 1;
+  if (!score.good_clipping)
+    type += 2;
+  if (!score.load_completed)
+    type += 3;
+  return type;
 }
 
 ThumbnailScore::ThumbnailScore()
     : boring_score(1.0),
       good_clipping(false),
       at_top(false),
+      load_completed(false),
       time_at_snapshot(Time::Now()),
       redirect_hops_from_dest(0) {
 }
@@ -44,6 +42,7 @@ ThumbnailScore::ThumbnailScore(double score, bool clipping, bool top)
     : boring_score(score),
       good_clipping(clipping),
       at_top(top),
+      load_completed(false),
       time_at_snapshot(Time::Now()),
       redirect_hops_from_dest(0) {
 }
@@ -53,6 +52,7 @@ ThumbnailScore::ThumbnailScore(double score, bool clipping, bool top,
     : boring_score(score),
       good_clipping(clipping),
       at_top(top),
+      load_completed(false),
       time_at_snapshot(time),
       redirect_hops_from_dest(0) {
 }
@@ -73,19 +73,20 @@ bool ThumbnailScore::Equals(const ThumbnailScore& rhs) const {
 
 std::string ThumbnailScore::ToString() const {
   return StringPrintf("boring_score: %f, at_top %d, good_clipping %d, "
+                      "load_completed: %d, "
                       "time_at_snapshot: %f, redirect_hops_from_dest: %d",
                       boring_score,
                       at_top,
                       good_clipping,
+                      load_completed,
                       time_at_snapshot.ToDoubleT(),
                       redirect_hops_from_dest);
 }
 
 bool ShouldReplaceThumbnailWith(const ThumbnailScore& current,
                                 const ThumbnailScore& replacement) {
-  int current_type = GetThumbnailType(current.good_clipping, current.at_top);
-  int replacement_type = GetThumbnailType(replacement.good_clipping,
-                                          replacement.at_top);
+  int current_type = GetThumbnailType(current);
+  int replacement_type = GetThumbnailType(replacement);
   if (replacement_type < current_type) {
     // If we have a better class of thumbnail, add it if it meets
     // certain minimum boringness.
@@ -131,7 +132,7 @@ bool ShouldReplaceThumbnailWith(const ThumbnailScore& current,
 bool ThumbnailScore::ShouldConsiderUpdating() {
   const TimeDelta time_elapsed = Time::Now() - time_at_snapshot;
   if (time_elapsed < kUpdateThumbnailTime &&
-      good_clipping && at_top) {
+      good_clipping && at_top && load_completed) {
     // The current thumbnail is new and has good properties.
     return false;
   }
diff --git a/chrome/common/thumbnail_score.h b/chrome/common/thumbnail_score.h
index d35d7d8..0a0650b 100644
--- a/chrome/common/thumbnail_score.h
+++ b/chrome/common/thumbnail_score.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -57,6 +57,11 @@ struct ThumbnailScore {
   // thumbnails with |at_top| set to false.
   bool at_top;
 
+  // Whether this thumbnail was taken after load was completed.
+  // Thumbnails taken while page loading may only contain partial
+  // contents.
+  bool load_completed;
+
   // Record the time when a thumbnail was taken. This is used to make
   // sure thumbnails are kept fresh.
   base::Time time_at_snapshot;
diff --git a/chrome/common/thumbnail_score_unittest.cc b/chrome/common/thumbnail_score_unittest.cc
index 2fe6632..5e3151f 100644
--- a/chrome/common/thumbnail_score_unittest.cc
+++ b/chrome/common/thumbnail_score_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -66,8 +66,13 @@ TEST(ThumbnailScoreTest, ShouldConsiderUpdating) {
   score.good_clipping = true;
   EXPECT_TRUE(score.ShouldConsiderUpdating());
 
-  // at_top is important. Finally, the thumbnail is new and interesting enough.
+  // at_top is important, but still not enough.
   score.at_top = true;
+  EXPECT_TRUE(score.ShouldConsiderUpdating());
+
+  // load_completed is important. Finally, the thumbnail is new and
+  // interesting enough.
+  score.load_completed = true;
   EXPECT_FALSE(score.ShouldConsiderUpdating());
 
   // Make it very boring, but it won't change the result. The boring score