Relanding this patch as the previous one broke valgrind tests and reliability tests.

The ResourceDispatcher object does not honor the deferred load flag for a request correctly. If this flag is set it correctly queues up any subsequent responses.

When the flag is reset it starts dispatching these responses. If the deferred flag is set in the context
of these responses it continues to dispatch further responses, which is not correct.

Fix is to check if the deferred flag for a request is set in the context of a response and stop dispatching if yes.

This fixes bug http://code.google.com/p/chromium/issues/detail?id=19931 and could potentially explain
this http://code.google.com/p/chromium/issues/detail?id=19393

Bug=19931
Test= Covered by unit test.

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

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

2 files changed, 132 insertions, 6 deletions

diff --git a/chrome/common/resource_dispatcher.cc b/chrome/common/resource_dispatcher.cc
index d0a90a5..82bd5af 100644
--- a/chrome/common/resource_dispatcher.cc
+++ b/chrome/common/resource_dispatcher.cc
@@ -528,6 +528,18 @@ void ResourceDispatcher::FlushDeferredMessages(int request_id) {
     q.pop_front();
     DispatchMessage(*m);
     delete m;
+    // If this request is deferred in the context of the above message, then
+    // we should honor the same and stop dispatching further messages.
+    // We need to find the request again in the list as it may have completed
+    // by now and the request_info instance above may be invalid.
+    PendingRequestList::iterator index = pending_requests_.find(request_id);
+    if (index != pending_requests_.end()) {
+      PendingRequestInfo& pending_request = index->second;
+      if (pending_request.is_deferred) {
+        pending_request.deferred_message_queue.swap(q);
+        return;
+      }
+    }
   }
 }
 
diff --git a/chrome/common/resource_dispatcher_unittest.cc b/chrome/common/resource_dispatcher_unittest.cc
index a270be3..39464ee 100644
--- a/chrome/common/resource_dispatcher_unittest.cc
+++ b/chrome/common/resource_dispatcher_unittest.cc
@@ -5,6 +5,7 @@
 #include <string>
 #include <vector>
 
+#include "base/message_loop.h"
 #include "base/process.h"
 #include "base/scoped_ptr.h"
 #include "chrome/common/filter_policy.h"
@@ -142,6 +143,14 @@ class ResourceDispatcherTest : public testing::Test,
     dispatcher_.reset();
   }
 
+  ResourceLoaderBridge* CreateBridge() {
+    ResourceLoaderBridge* bridge = dispatcher_->CreateBridge(
+        "GET", GURL(test_page_url), GURL(test_page_url), GURL(), "null",
+        "null", std::string(), 0, 0, ResourceType::SUB_RESOURCE, 0,
+        appcache::kNoHostId, MSG_ROUTING_CONTROL);
+    return bridge;
+  }
+
   std::vector<IPC::Message> message_queue_;
   static scoped_ptr<ResourceDispatcher> dispatcher_;
 };
@@ -152,12 +161,7 @@ scoped_ptr<ResourceDispatcher> ResourceDispatcherTest::dispatcher_;
 // Does a simple request and tests that the correct data is received.
 TEST_F(ResourceDispatcherTest, RoundTrip) {
   TestRequestCallback callback;
-  ResourceLoaderBridge* bridge =
-    dispatcher_->CreateBridge("GET", GURL(test_page_url), GURL(test_page_url),
-                              GURL(), "null", "null", std::string(), 0, 0,
-                              ResourceType::SUB_RESOURCE, 0,
-                              appcache::kNoHostId,
-                              MSG_ROUTING_CONTROL);
+  ResourceLoaderBridge* bridge = CreateBridge();
 
   bridge->Start(&callback);
 
@@ -188,3 +192,113 @@ TEST_F(ResourceDispatcherTest, Cookies) {
 TEST_F(ResourceDispatcherTest, SerializedPostData) {
   // FIXME
 }
+
+// This class provides functionality to validate whether the ResourceDispatcher
+// object honors the deferred loading contract correctly, i.e. if deferred
+// loading is enabled it should queue up any responses received. If deferred
+// loading is enabled/disabled in the context of a dispatched message, other
+// queued messages should not be dispatched until deferred load is turned off.
+class DeferredResourceLoadingTest : public ResourceDispatcherTest,
+                                    public ResourceLoaderBridge::Peer {
+ public:
+  DeferredResourceLoadingTest()
+      : defer_loading_(false) {
+  }
+
+  virtual bool Send(IPC::Message* msg) {
+    delete msg;
+    return true;
+  }
+
+  void InitMessages() {
+    set_defer_loading(true);
+
+    ResourceResponseHead response_head;
+    response_head.status.set_status(URLRequestStatus::SUCCESS);
+
+    IPC::Message* response_message =
+        new ViewMsg_Resource_ReceivedResponse(0, 0, response_head);
+
+    dispatcher_->OnMessageReceived(*response_message);
+
+    delete response_message;
+
+    response_message =
+        new ViewMsg_Resource_DataReceived(0, 0, shared_handle_.handle(), 100);
+
+    dispatcher_->OnMessageReceived(*response_message);
+
+    delete response_message;
+
+    set_defer_loading(false);
+  }
+
+  // ResourceLoaderBridge::Peer methods.
+  virtual void OnReceivedResponse(
+      const ResourceLoaderBridge::ResponseInfo& info,
+      bool content_filtered) {
+    EXPECT_EQ(defer_loading_, false);
+    set_defer_loading(true);
+  }
+
+  virtual bool OnReceivedRedirect(
+      const GURL& new_url,
+      const ResourceLoaderBridge::ResponseInfo& info) {
+    return true;
+  }
+
+  virtual void OnReceivedData(const char* data, int len) {
+    EXPECT_EQ(defer_loading_, false);
+    set_defer_loading(false);
+  }
+
+  virtual void OnUploadProgress(uint64 position, uint64 size) {
+  }
+
+  virtual void OnCompletedRequest(const URLRequestStatus& status,
+                                  const std::string& security_info) {
+  }
+
+  virtual std::string GetURLForDebugging() {
+    return std::string();
+  }
+
+ protected:
+  virtual void SetUp() {
+    EXPECT_EQ(true, shared_handle_.Create(L"DeferredResourceLoaderTest", false,
+                                          false, 100));
+    ResourceDispatcherTest::SetUp();
+  }
+
+  virtual void TearDown() {
+    shared_handle_.Close();
+    ResourceDispatcherTest::TearDown();
+  }
+
+ private:
+  void set_defer_loading(bool defer) {
+    defer_loading_ = defer;
+    dispatcher_->SetDefersLoading(0, defer);
+  }
+
+  bool defer_loading() const {
+    return defer_loading_;
+  }
+
+  bool defer_loading_;
+  base::SharedMemory shared_handle_;
+};
+
+TEST_F(DeferredResourceLoadingTest, DeferredLoadTest) {
+  MessageLoop message_loop(MessageLoop::TYPE_IO);
+
+  ResourceLoaderBridge* bridge = CreateBridge();
+
+  bridge->Start(this);
+  InitMessages();
+
+  // Dispatch deferred messages.
+  message_loop.RunAllPending();
+  delete bridge;
+}
+