Merge 102541 - Change PhishingDOMFeatureExtractor to cache the WebDocument rather than a WebFrame...

chrome/renderer/safe_browsing/phishing_dom_feature_extractor.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/renderer/safe_browsing/phishing_dom_feature_extractor.h"
 
 #include "base/compiler_specific.h"
 #include "base/hash_tables.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "chrome/renderer/safe_browsing/feature_extractor_clock.h"
 #include "chrome/renderer/safe_browsing/features.h"
 #include "content/renderer/render_view.h"
 #include "net/base/registry_controlled_domain.h"
-#include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebElement.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebNodeCollection.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebString.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h"
 
 namespace safe_browsing {
 
 // This time should be short enough that it doesn't noticeably disrupt the
 // user's interaction with the page.
@@ skipping 91 matching lines @@
   // starting a new extraction, so DCHECK this.
   CheckNoPendingExtraction();
   // However, in an opt build, we will go ahead and clean up the pending
   // extraction so that we can start in a known state.
   CancelPendingExtraction();
 
   features_ = features;
   done_callback_.reset(done_callback);
 
   page_feature_state_.reset(new PageFeatureState(clock_->Now()));
+  WebKit::WebView* web_view = render_view_->webview();
+  if (web_view && web_view->mainFrame()) {
+    cur_document_ = web_view->mainFrame()->document();
+  }
+
   MessageLoop::current()->PostTask(
       FROM_HERE,
       method_factory_.NewRunnableMethod(
           &PhishingDOMFeatureExtractor::ExtractFeaturesWithTimeout));
 }
 
 void PhishingDOMFeatureExtractor::CancelPendingExtraction() {
   // Cancel any pending callbacks, and clear our state.
   method_factory_.RevokeAll();
   Clear();
 }
 
 void PhishingDOMFeatureExtractor::ExtractFeaturesWithTimeout() {
   DCHECK(page_feature_state_.get());
   ++page_feature_state_->num_iterations;
   base::TimeTicks current_chunk_start_time = clock_->Now();
 
-  if (!cur_frame_) {
-    WebKit::WebView* web_view = render_view_->webview();
-    if (!web_view) {
-      RunCallback(false);  // The WebView is going away.
-      return;
-    }
-    cur_frame_ = web_view->mainFrame();
+  if (cur_document_.isNull()) {
+    // This will only happen if we weren't able to get the document for the
+    // main frame.  We'll treat this as an extraction failure.
+    RunCallback(false);
+    return;
   }
 
   int num_elements = 0;
-  for (; cur_frame_;
-       cur_frame_ = cur_frame_->traverseNext(false /* don't wrap around */)) {
+  for (; !cur_document_.isNull(); cur_document_ = GetNextDocument()) {
     WebKit::WebNode cur_node;
     if (cur_frame_data_.get()) {
       // We're resuming traversal of a frame, so just advance to the next node.
       cur_node = cur_frame_data_->elements.nextItem();
       // When we resume the traversal, the first call to nextItem() potentially
       // has to walk through the document again from the beginning, if it was
       // modified between our chunks of work.  Log how long this takes, so we
       // can tell if it's too slow.
       UMA_HISTOGRAM_TIMES("SBClientPhishing.DOMFeatureResumeTime",
                           clock_->Now() - current_chunk_start_time);
@@ skipping 175 matching lines @@
 }
 
 void PhishingDOMFeatureExtractor::HandleScript(
     const WebKit::WebElement& element) {
   ++page_feature_state_->num_script_tags;
 }
 
 void PhishingDOMFeatureExtractor::CheckNoPendingExtraction() {
   DCHECK(!done_callback_.get());
   DCHECK(!cur_frame_data_.get());
-  DCHECK(!cur_frame_);
-  if (done_callback_.get() || cur_frame_data_.get() || cur_frame_) {
+  DCHECK(cur_document_.isNull());
+  if (done_callback_.get() || cur_frame_data_.get() ||
+      !cur_document_.isNull()) {
     LOG(ERROR) << "Extraction in progress, missing call to "
                << "CancelPendingExtraction";
   }
 }
 
 void PhishingDOMFeatureExtractor::RunCallback(bool success) {
   // Record some timing stats that we can use to evaluate feature extraction
   // performance.  These include both successful and failed extractions.
   DCHECK(page_feature_state_.get());
   UMA_HISTOGRAM_COUNTS("SBClientPhishing.DOMFeatureIterations",
                        page_feature_state_->num_iterations);
   UMA_HISTOGRAM_TIMES("SBClientPhishing.DOMFeatureTotalTime",
                       clock_->Now() - page_feature_state_->start_time);
 
   DCHECK(done_callback_.get());
   done_callback_->Run(success);
   Clear();
 }
 
 void PhishingDOMFeatureExtractor::Clear() {
   features_ = NULL;
   done_callback_.reset(NULL);
   cur_frame_data_.reset(NULL);
-  cur_frame_ = NULL;
+  cur_document_.reset();
 }
 
 bool PhishingDOMFeatureExtractor::ResetFrameData() {
-  DCHECK(cur_frame_);
+  DCHECK(!cur_document_.isNull());
   DCHECK(!cur_frame_data_.get());
 
-  WebKit::WebDocument doc = cur_frame_->document();
-  if (doc.isNull()) {
-    return false;
-  }
   cur_frame_data_.reset(new FrameData());
-  cur_frame_data_->elements = doc.all();
+  cur_frame_data_->elements = cur_document_.all();
   cur_frame_data_->domain =
       net::RegistryControlledDomainService::GetDomainAndRegistry(
-          cur_frame_->document().url());
+          cur_document_.url());
   return true;
 }
 
+WebKit::WebDocument PhishingDOMFeatureExtractor::GetNextDocument() {
+  DCHECK(!cur_document_.isNull());
+  WebKit::WebFrame* frame = cur_document_.frame();
+  // Advance to the next frame that contains a document, with no wrapping.
+  if (frame) {
+    while ((frame = frame->traverseNext(false))) {
+      if (!frame->document().isNull()) {
+        return frame->document();
+      }
+    }
+  } else {
+    // Keep track of how often frame traversal got "stuck" due to the
+    // current subdocument getting removed from the frame tree.
+    UMA_HISTOGRAM_COUNTS("SBClientPhishing.DOMFeatureFrameRemoved", 1);
+  }
+  return WebKit::WebDocument();
+}
+
 bool PhishingDOMFeatureExtractor::IsExternalDomain(const GURL& url,
                                                    std::string* domain) const {
   DCHECK(domain);
   DCHECK(cur_frame_data_.get());
 
   if (cur_frame_data_->domain.empty()) {
     return false;
   }
 
   // TODO(bryner): Ensure that the url encoding is consistent with the features
@@ skipping 69 matching lines @@
   // Record number of script tags (discretized for numerical stability.)
   if (page_feature_state_->num_script_tags > 1) {
     features_->AddBooleanFeature(features::kPageNumScriptTagsGTOne);
     if (page_feature_state_->num_script_tags > 6) {
       features_->AddBooleanFeature(features::kPageNumScriptTagsGTSix);
     }
   }
 }
 
 }  // namespace safe_browsing