Fix ChromeRenderViewObserver::DidStopLoading to be OOPIF compatible.

chrome/renderer/chrome_render_view_observer.cc

 // Copyright (c) 2012 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/chrome_render_view_observer.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 316 matching lines @@
     for (size_t i = 0; i < webui_javascript_.size(); ++i) {
       render_view()->GetMainRenderFrame()->ExecuteJavaScript(
           webui_javascript_[i]);
     }
     webui_javascript_.clear();
   }
 }
 
 void ChromeRenderViewObserver::DidStopLoading() {
   WebFrame* main_frame = render_view()->GetWebView()->mainFrame();
-  GURL osdd_url = main_frame->document().openSearchDescriptionURL();
-  if (!osdd_url.is_empty()) {
-    Send(new ChromeViewHostMsg_PageHasOSDD(
-        routing_id(), main_frame->document().url(), osdd_url,
-        search_provider::AUTODETECTED_PROVIDER));
+
+  // Only WebLocalFrames host the document associated with a frame and need to
+  // process meta or OSDD tags.
+  if (main_frame->isWebLocalFrame()) {
+    GURL osdd_url = main_frame->document().openSearchDescriptionURL();
+    if (!osdd_url.is_empty()) {
+      Send(new ChromeViewHostMsg_PageHasOSDD(
+          routing_id(), main_frame->document().url(), osdd_url,
+          search_provider::AUTODETECTED_PROVIDER));
+    }
+
+    // Don't capture pages including refresh meta tag.
+    if (HasRefreshMetaTag(main_frame))
+      return;
   }
 
-  // Don't capture pages including refresh meta tag.
-  if (HasRefreshMetaTag(main_frame))
-    return;
-
   CapturePageInfoLater(

[Lei Zhang, 2014/12/16 23:46:57]

I think you want this to be for WebLocalFrame only as well? This eventually goes
to ChromeRenderViewObserver::CapturePageInfo(), which currently first checks and
ignores WebRemoteFrames.

[nasko, 2014/12/16 23:48:59]

I'm not certain and the CapturePageInfo already has a check for remote frames,
which is tagged separately. Do you think it is worth just returning early then?

[Lei Zhang, 2014/12/17 00:14:20]

It looks like creis@ has plans to move the CapturePageInfo() code to the browser
process eventually, so I think the whole thing can go inside the if
(isWebLocalFrame()) block. The HasRefreshMetaTag() and CapturePageInfoLater()
calls are related, so it feels weird to separate them.

I don't know if the OSDD portion is correct. Do we care about OSDD tags in
remote frames?

       false,  // preliminary_capture
       base::TimeDelta::FromMilliseconds(
           render_view()->GetContentStateImmediately() ?
               0 : kDelayForCaptureMs));
 }
 
 void ChromeRenderViewObserver::DidCommitProvisionalLoad(
     WebLocalFrame* frame, bool is_new_navigation) {
   // Don't capture pages being not new, or including refresh meta tag.
   if (!is_new_navigation || HasRefreshMetaTag(frame))
@@ skipping 113 matching lines @@
     WebElement element = node.to<WebElement>();
     if (!element.hasHTMLTagName(tag_name))
       continue;
     WebString value = element.getAttribute(attribute_name);
     if (value.isNull() || !LowerCaseEqualsASCII(value, "refresh"))
       continue;
     return true;
   }
   return false;
 }