Reset content settings caches during provisional load start instead of commit.

chrome/renderer/content_settings_observer.cc

Index: chrome/renderer/content_settings_observer.cc
diff --git a/chrome/renderer/content_settings_observer.cc b/chrome/renderer/content_settings_observer.cc
index 2e48be56122421ad169c48629000ce9051e82c19..5913e4175c6cc542d1ab00e19e9b4bbb6f704cc1 100644
--- a/chrome/renderer/content_settings_observer.cc
+++ b/chrome/renderer/content_settings_observer.cc
@@ -8,7 +8,6 @@
 #include "base/metrics/histogram.h"
 #include "components/content_settings/content/common/content_settings_messages.h"
 #include "content/public/common/url_constants.h"
-#include "content/public/renderer/document_state.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
 #include "third_party/WebKit/public/platform/URLConversion.h"
@@ -39,8 +38,6 @@ using blink::WebSecurityOrigin;
 using blink::WebString;
 using blink::WebURL;
 using blink::WebView;
-using content::DocumentState;
-using content::NavigationState;
 
 namespace {
 
@@ -227,23 +224,26 @@ bool ContentSettingsObserver::OnMessageReceived(const IPC::Message& message) {
   return false;
 }
 
+void ContentSettingsObserver::DidCreateNewDocument() {

[jochen (gone - plz use gerrit), 2016/03/31 15:26:19]

didCreateNewDocument is also invoked when somebody invokes document.open()

[meacer, 2016/04/01 19:06:56]

Did you mean DidCreateDocumentElement? That's the one I'm seeing with
document.open().

window.open() does invoke didCreateNewDocument but it's for the newly opened
url.

+  // Clear "block" flags for the new page. This needs to happen before any of
+  // |allowScript()|, |allowScriptFromSource()|, |allowImage()|, or
+  // |allowPlugins()| is called for the new page so that these functions can
+  // correctly detect that a piece of content flipped from "not blocked" to
+  // "blocked".
+  // This also needs to happen before the document load begins, as parsing the
+  // document can trigger calls to ScriptController::canExecuteScripts.
+  // There is no need to check for same page navigations here as those don't
+  // trigger DidStartProvisionalLoad.
+  ClearBlockedContentSettings();
+  temporarily_allowed_plugins_.clear();
+}
+
 void ContentSettingsObserver::DidCommitProvisionalLoad(
     bool is_new_navigation,
     bool is_same_page_navigation) {
   WebFrame* frame = render_frame()->GetWebFrame();
   if (frame->parent())
     return;  // Not a top-level navigation.
-
-  if (!is_same_page_navigation) {
-    // Clear "block" flags for the new page. This needs to happen before any of
-    // |allowScript()|, |allowScriptFromSource()|, |allowImage()|, or
-    // |allowPlugins()| is called for the new page so that these functions can
-    // correctly detect that a piece of content flipped from "not blocked" to
-    // "blocked".
-    ClearBlockedContentSettings();
-    temporarily_allowed_plugins_.clear();
-  }
-
   GURL url = frame->document().url();
   // If we start failing this DCHECK, please makes sure we don't regress
   // this bug: http://code.google.com/p/chromium/issues/detail?id=79304