Send "content blocked" ipc messages more reliably.

I believe that the bug happened in this sequence of events:

1.) JS gets blocked on the web page, icon appears. The renderer (correctly) thinks js is blocked.
2.) User hits "reload"
3.) RenderView::didCommitProvisionalLoad() calls RenderView::UpdateURL() which sends ViewHostMsg_FrameNavigate to the browser which clears the content settings images in the location bar
4.) RenderView::allowScripts() gets called during page loading. The renderer still thinks that js is blocked and hence doesn't notify the browser.
5.) RenderView::didReceiveResponse() clears the content blocked flags (too late)

This CL moves the content blocked flags into navigationstate, which should prevent this "race" because there's one new navigationstate per page.

BUG=35011
TEST=See bug.

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

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

4 files changed, 23 insertions, 22 deletions

diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 704428d..3d66ebf 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -1282,9 +1282,7 @@ TabContents* TabContents::CloneAndMakePhantom() {
 
 // Resets the |content_blocked_| array.
 void TabContents::ClearBlockedContentSettings() {
-  DCHECK_EQ(static_cast<size_t>(CONTENT_SETTINGS_NUM_TYPES),
-            arraysize(content_blocked_));
-  for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i)
+  for (size_t i = 0; i < arraysize(content_blocked_); ++i)
     content_blocked_[i] = false;
 }
 
diff --git a/chrome/renderer/navigation_state.h b/chrome/renderer/navigation_state.h
index d17b542..d3d2418 100644
--- a/chrome/renderer/navigation_state.h
+++ b/chrome/renderer/navigation_state.h
@@ -7,6 +7,7 @@
 
 #include "base/scoped_ptr.h"
 #include "base/time.h"
+#include "chrome/common/content_settings.h"
 #include "chrome/common/page_transition_types.h"
 #include "chrome/renderer/user_script_idle_scheduler.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebDataSource.h"
@@ -215,6 +216,13 @@ class NavigationState : public WebKit::WebDataSource::ExtraData {
   void set_was_translated(bool value) { was_translated_ = value; }
   bool was_translated() const { return was_translated_; }
 
+  // Whether content has been blocked.
+  bool is_content_blocked(ContentSettingsType settings_type) {
+    return content_blocked_[settings_type];
+  }
+  void set_content_blocked(ContentSettingsType settings_type, bool is_blocked) {
+    content_blocked_[settings_type] = is_blocked;
+  }
  private:
   NavigationState(PageTransition::Type transition_type,
                   const base::Time& request_time,
@@ -233,6 +241,8 @@ class NavigationState : public WebKit::WebDataSource::ExtraData {
         user_script_idle_scheduler_(NULL),
         was_fetched_via_spdy_(false),
         was_translated_(false) {
+    for (size_t i = 0; i < arraysize(content_blocked_); ++i)
+      content_blocked_[i] = false;
   }
 
   PageTransition::Type transition_type_;
@@ -265,6 +275,9 @@ class NavigationState : public WebKit::WebDataSource::ExtraData {
 
   bool was_translated_;
 
+  // Stores if images, scripts, and plugins have actually been blocked.
+  bool content_blocked_[CONTENT_SETTINGS_NUM_TYPES];
+
   DISALLOW_COPY_AND_ASSIGN(NavigationState);
 };
 
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index c345999..f4f3a2d 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -314,7 +314,6 @@ RenderView::RenderView(RenderThreadBase* render_thread,
       webkit_preferences_(webkit_preferences),
       session_storage_namespace_id_(session_storage_namespace_id),
       ALLOW_THIS_IN_INITIALIZER_LIST(text_translator_(this)) {
-  ClearBlockedContentSettings();
   page_translator_.reset(new PageTranslator(&text_translator_, this));
 }
 
@@ -1227,6 +1226,10 @@ void RenderView::UpdateURL(WebFrame* frame) {
     UMA_HISTOGRAM_COUNTS_10000("Memory.GlyphPagesPerLoad",
                                webkit_glue::GetGlyphPageCount());
 
+    // This message needs to be sent before any of allowScripts(),
+    // allowImages(), allowPlugins() is called for the new page, so that when
+    // these functions send a ViewHostMsg_ContentBlocked message, it arrives
+    // after the ViewHostMsg_FrameNavigate message.
     Send(new ViewHostMsg_FrameNavigate(routing_id_, params));
   } else {
     // Subframe navigation: the type depends on whether this navigation
@@ -2617,8 +2620,6 @@ void RenderView::didReceiveResponse(
   if (!frame->provisionalDataSource() || frame->parent())
     return;
 
-  // A new page is about to be loaded. Clear "block" flags.
-  ClearBlockedContentSettings();
   // If we are in view source mode, then just let the user see the source of
   // the server's 404 error page.
   if (frame->isViewSourceModeEnabled())
@@ -3210,8 +3211,10 @@ bool RenderView::AllowContentType(ContentSettingsType settings_type,
   // CONTENT_SETTING_ASK is only valid for cookies.
   if (current_content_settings_.settings[settings_type] ==
       CONTENT_SETTING_BLOCK) {
-    if (!content_blocked_[settings_type]) {
-      content_blocked_[settings_type] = true;
+    NavigationState* navigation_state =
+        NavigationState::FromDataSource(webview()->mainFrame()->dataSource());
+    if (!navigation_state->is_content_blocked(settings_type)) {
+      navigation_state->set_content_blocked(settings_type, true);
       Send(new ViewHostMsg_ContentBlocked(routing_id_, settings_type));
     }
     return false;
@@ -3219,13 +3222,6 @@ bool RenderView::AllowContentType(ContentSettingsType settings_type,
   return true;
 }
 
-void RenderView::ClearBlockedContentSettings() {
-  DCHECK_EQ(static_cast<size_t>(CONTENT_SETTINGS_NUM_TYPES),
-            arraysize(content_blocked_));
-  for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i)
-    content_blocked_[i] = false;
-}
-
 void RenderView::DnsPrefetch(const std::vector<std::string>& host_names) {
   Send(new ViewHostMsg_DnsPrefetch(host_names));
 }
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index 0066bf3..18a0875 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -839,9 +839,6 @@ class RenderView : public RenderWidget,
   bool AllowContentType(ContentSettingsType settings_type,
                         bool enabled_per_settings);
 
-  // Resets the |content_blocked_| array.
-  void ClearBlockedContentSettings();
-
   // Bitwise-ORed set of extra bindings that have been enabled.  See
   // BindingsPolicy for details.
   int enabled_bindings_;
@@ -1028,7 +1025,7 @@ class RenderView : public RenderWidget,
   // The text selection the last time DidChangeSelection got called.
   std::string last_selection_;
 
-  // Hopds a reference to the service which provides desktop notifications.
+  // Holds a reference to the service which provides desktop notifications.
   scoped_ptr<NotificationProvider> notification_provider_;
 
   // Holds state pertaining to a navigation that we initiated.  This is held by
@@ -1083,9 +1080,6 @@ class RenderView : public RenderWidget,
   // Stores if loading of images, scripts, and plugins is allowed.
   ContentSettings current_content_settings_;
 
-  // Stores if images, scripts, and plugins have actually been blocked.
-  bool content_blocked_[CONTENT_SETTINGS_NUM_TYPES];
-
   // The SessionStorage namespace that we're assigned to has an ID, and that ID
   // is passed to us upon creation.  WebKit asks for this ID upon first use and
   // uses it whenever asking the browser process to allocate new storage areas.