Send "content blocked" ipc messages even more reliably.

This effectively reverts patch set 6 from http://codereview.chromium.org/596039 and adds patch set 1 instead.

Also add a regression test to make sure the bug is actually fixed.

The problem with using navigation state was that allowScripts() was still called before the FrameNavigate message was sent.

BUG=35011
TEST=See bug.

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

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

4 files changed, 73 insertions, 18 deletions

diff --git a/chrome/renderer/navigation_state.h b/chrome/renderer/navigation_state.h
index d3d2418..d17b542 100644
--- a/chrome/renderer/navigation_state.h
+++ b/chrome/renderer/navigation_state.h
@@ -7,7 +7,6 @@
 
 #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"
@@ -216,13 +215,6 @@ 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,
@@ -241,8 +233,6 @@ 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_;
@@ -275,9 +265,6 @@ 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 f4f3a2d..c086f25 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -314,6 +314,7 @@ 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));
 }
 
@@ -1160,6 +1161,12 @@ void RenderView::UpdateURL(WebFrame* frame) {
   if (!frame->parent()) {
     // Top-level navigation.
 
+    // Clear "block" flags for the new page. This needs to happen before any of
+    // allowScripts(), allowImages(), 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();
+
     // Set content settings.
     HostContentSettings::iterator host_content_settings =
         host_content_settings_.find(GURL(request.url()).host());
@@ -3211,10 +3218,8 @@ bool RenderView::AllowContentType(ContentSettingsType settings_type,
   // CONTENT_SETTING_ASK is only valid for cookies.
   if (current_content_settings_.settings[settings_type] ==
       CONTENT_SETTING_BLOCK) {
-    NavigationState* navigation_state =
-        NavigationState::FromDataSource(webview()->mainFrame()->dataSource());
-    if (!navigation_state->is_content_blocked(settings_type)) {
-      navigation_state->set_content_blocked(settings_type, true);
+    if (!content_blocked_[settings_type]) {
+      content_blocked_[settings_type] = true;
       Send(new ViewHostMsg_ContentBlocked(routing_id_, settings_type));
     }
     return false;
@@ -3222,6 +3227,11 @@ bool RenderView::AllowContentType(ContentSettingsType settings_type,
   return true;
 }
 
+void RenderView::ClearBlockedContentSettings() {
+  for (size_t i = 0; i < arraysize(content_blocked_); ++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 18a0875..f84685a 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -505,6 +505,7 @@ class RenderView : public RenderWidget,
 #if defined(OS_MACOSX)
   FRIEND_TEST(RenderViewTest, MacTestCmdUp);
 #endif
+  FRIEND_TEST(RenderViewTest, JSBlockSentAfterPageLoad);
 
   typedef std::map<std::string, ContentSettings> HostContentSettings;
   typedef std::map<std::string, int> HostZoomLevels;
@@ -839,6 +840,9 @@ 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_;
@@ -1080,6 +1084,9 @@ 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.
diff --git a/chrome/renderer/render_view_unittest.cc b/chrome/renderer/render_view_unittest.cc
index d5a4335..f22f772 100644
--- a/chrome/renderer/render_view_unittest.cc
+++ b/chrome/renderer/render_view_unittest.cc
@@ -5,6 +5,7 @@
 #include "app/gfx/codec/jpeg_codec.h"
 #include "base/file_util.h"
 #include "base/shared_memory.h"
+#include "chrome/common/content_settings.h"
 #include "chrome/common/native_web_keyboard_event.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/renderer/print_web_view_helper.h"
@@ -535,7 +536,7 @@ TEST_F(RenderViewTest, OnPrintPageAsBitmap) {
                                      gfx::JPEGCodec::FORMAT_RGBA,
                                      &decoded, &w, &h));
 
-  // Check if its not 100% white.
+  // Check if it's not 100% white.
   bool is_white = true;
   for (int y = 0; y < h; y++) {
     for (int x = 0; x < w; x++) {
@@ -924,3 +925,53 @@ TEST_F(RenderViewTest, DidFailProvisionalLoadWithErrorForCancellation) {
   // Frame should stay in view-source mode.
   EXPECT_TRUE(web_frame->isViewSourceModeEnabled());
 }
+
+// Regression test for http://crbug.com/35011
+TEST_F(RenderViewTest, JSBlockSentAfterPageLoad) {
+  // 1. Load page with JS.
+  std::string html = "<html>"
+           "<head>"
+           "<script>document.createElement('div');</script>"
+           "</head>"
+           "<body>"
+           "</body>"
+           "</html>";
+  render_thread_.sink().ClearMessages();
+  LoadHTML(html.c_str());
+
+  // 2. Block JavaScript.
+  ContentSettings settings;
+  for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i)
+    settings.settings[i] = CONTENT_SETTING_ALLOW;
+  settings.settings[CONTENT_SETTINGS_TYPE_JAVASCRIPT] = CONTENT_SETTING_BLOCK;
+  view_->SetContentSettings(settings);
+
+  // Make sure no pending messages are in the queue.
+  ProcessPendingMessages();
+  render_thread_.sink().ClearMessages();
+
+  // 3. Reload page.
+  ViewMsg_Navigate_Params params;
+  std::string url_str = "data:text/html;charset=utf-8,";
+  url_str.append(html);
+  GURL url(url_str);
+  params.url = url;
+  params.navigation_type = ViewMsg_Navigate_Params::RELOAD;
+  view_->OnNavigate(params);
+  ProcessPendingMessages();
+
+  // 4. Verify that the notification that javascript was blocked is sent after
+  //    the navigation notifiction is sent.
+  int navigation_index = -1;
+  int block_index = -1;
+  for (size_t i = 0; i < render_thread_.sink().message_count(); ++i) {
+    const IPC::Message* msg = render_thread_.sink().GetMessageAt(i);
+    if (msg->type() == ViewHostMsg_FrameNavigate::ID)
+      navigation_index = i;
+    if (msg->type() == ViewHostMsg_ContentBlocked::ID)
+      block_index = i;
+  }
+  EXPECT_NE(-1, navigation_index);
+  EXPECT_NE(-1, block_index);
+  EXPECT_LT(navigation_index, block_index);
+}