Fix crash in PhishingTermFeatureExtractor due to negative_word_cache_ not always being cleared.

Same as: http://codereview.chromium.org/7637027/

BUG=92676
TEST=Ran unittest under valgrind.

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

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

2 files changed, 58 insertions, 4 deletions

diff --git a/chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc b/chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc
index 2c4c990..3a13d31 100644
--- a/chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc
+++ b/chrome/renderer/safe_browsing/phishing_term_feature_extractor.cc
@@ -177,7 +177,6 @@ void PhishingTermFeatureExtractor::ExtractFeaturesWithTimeout() {
         DLOG(ERROR) << "Feature extraction took too long, giving up";
         // We expect this to happen infrequently, so record when it does.
         UMA_HISTOGRAM_COUNTS("SBClientPhishing.TermFeatureTimeout", 1);
-        negative_word_cache_.Clear();
         RunCallback(false);
         return;
       }
@@ -201,9 +200,6 @@ void PhishingTermFeatureExtractor::ExtractFeaturesWithTimeout() {
       // Otherwise, continue.
     }
   }
-  // We need to clear the cache because the data that it depends on (page_text_)
-  // is going away.
-  negative_word_cache_.Clear();
   RunCallback(true);
 }
 
@@ -299,6 +295,7 @@ void PhishingTermFeatureExtractor::Clear() {
   features_ = NULL;
   done_callback_.reset(NULL);
   state_.reset(NULL);
+  negative_word_cache_.Clear();
 }
 
 }  // namespace safe_browsing
diff --git a/chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc b/chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc
index 1b18bf1..c1cb5b3 100644
--- a/chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc
+++ b/chrome/renderer/safe_browsing/phishing_term_feature_extractor_unittest.cc
@@ -6,6 +6,7 @@
 
 #include <string>
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/hash_tables.h"
 #include "base/memory/scoped_ptr.h"
@@ -84,12 +85,29 @@ class PhishingTermFeatureExtractorTest : public ::testing::Test {
     return success_;
   }
 
+  void PartialExtractFeatures(const string16* page_text, FeatureMap* features) {
+    extractor_->ExtractFeatures(
+        page_text,
+        features,
+        NewCallback(this, &PhishingTermFeatureExtractorTest::ExtractionDone));
+    msg_loop_.PostTask(
+        FROM_HERE,
+        base::Bind(&PhishingTermFeatureExtractorTest::QuitExtraction,
+                   base::Unretained(this)));
+    msg_loop_.RunAllPending();
+  }
+
   // Completion callback for feature extraction.
   void ExtractionDone(bool success) {
     success_ = success;
     msg_loop_.Quit();
   }
 
+  void QuitExtraction() {
+    extractor_->CancelPendingExtraction();
+    msg_loop_.Quit();
+  }
+
   MessageLoop msg_loop_;
   MockFeatureExtractorClock clock_;
   scoped_ptr<PhishingTermFeatureExtractor> extractor_;
@@ -242,4 +260,43 @@ TEST_F(PhishingTermFeatureExtractorTest, Continuation) {
   EXPECT_FALSE(ExtractFeatures(&page_text, &features));
 }
 
+TEST_F(PhishingTermFeatureExtractorTest, PartialExtractionTest) {
+  scoped_ptr<string16> page_text(new string16(ASCIIToUTF16("one ")));
+  for (int i = 0; i < 28; ++i) {
+    page_text->append(ASCIIToUTF16(StringPrintf("%d ", i)));
+  }
+
+  base::TimeTicks now = base::TimeTicks::Now();
+  EXPECT_CALL(clock_, Now())
+      // Time check at the start of extraction.
+      .WillOnce(Return(now))
+      // Time check at the start of the first chunk of work.
+      .WillOnce(Return(now))
+      // Time check after the first 10 words. This should be greater than
+      // kMaxTimePerChunkMs so that we stop and schedule extraction for later.
+      .WillOnce(Return(now + base::TimeDelta::FromMilliseconds(30)));
+
+  FeatureMap features;
+  // Extract first 10 words then stop.
+  PartialExtractFeatures(page_text.get(), &features);
+
+  page_text.reset(new string16());
+  for (int i = 30; i < 58; ++i) {
+    page_text->append(ASCIIToUTF16(StringPrintf("%d ", i)));
+  }
+  page_text->append(ASCIIToUTF16("multi word test "));
+  features.Clear();
+
+  // This part doesn't exercise the extraction timing.
+  EXPECT_CALL(clock_, Now()).WillRepeatedly(Return(base::TimeTicks::Now()));
+
+  // Now extract normally and make sure nothing breaks.
+  EXPECT_TRUE(ExtractFeatures(page_text.get(), &features));
+
+  FeatureMap expected_features;
+  expected_features.AddBooleanFeature(features::kPageTerm +
+                                      std::string("multi word test"));
+  EXPECT_THAT(features.features(), ContainerEq(expected_features.features()));
+}
+
 }  // namespace safe_browsing