Fix HasSingleOrigin() so it properly handles double redirect & added unit test.

BUG=72625
TEST=BufferedResourceLoaderTest.HasSingleOrigin, SimpleDataSourceTest.HasSingleOrigin

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

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

7 files changed, 157 insertions, 14 deletions

diff --git a/webkit/glue/media/buffered_data_source.cc b/webkit/glue/media/buffered_data_source.cc
index 4449100..913d400 100644
--- a/webkit/glue/media/buffered_data_source.cc
+++ b/webkit/glue/media/buffered_data_source.cc
@@ -38,7 +38,6 @@ BufferedDataSource::BufferedDataSource(
       loaded_(false),
       streaming_(false),
       frame_(frame),
-      single_origin_(true),
       loader_(NULL),
       network_activity_(false),
       initialize_callback_(NULL),
@@ -173,7 +172,7 @@ bool BufferedDataSource::IsStreaming() {
 
 bool BufferedDataSource::HasSingleOrigin() {
   DCHECK(MessageLoop::current() == render_loop_);
-  return single_origin_;
+  return loader_.get() ? loader_->HasSingleOrigin() : true;
 }
 
 void BufferedDataSource::Abort() {
@@ -406,9 +405,6 @@ void BufferedDataSource::HttpInitialStartCallback(int error) {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(loader_.get());
 
-  // Check if the request ended up at a different origin via redirect.
-  single_origin_ = url_.GetOrigin() == loader_->url().GetOrigin();
-
   int64 instance_size = loader_->instance_size();
   bool partial_response = loader_->partial_response();
   bool success = error == net::OK;
@@ -475,9 +471,6 @@ void BufferedDataSource::NonHttpInitialStartCallback(int error) {
   DCHECK(MessageLoop::current() == render_loop_);
   DCHECK(loader_.get());
 
-  // Check if the request ended up at a different origin via redirect.
-  single_origin_ = url_.GetOrigin() == loader_->url().GetOrigin();
-
   int64 instance_size = loader_->instance_size();
   bool success = error == net::OK && instance_size != kPositionNotSpecified;
 
diff --git a/webkit/glue/media/buffered_data_source.h b/webkit/glue/media/buffered_data_source.h
index 25a565a..895b259 100644
--- a/webkit/glue/media/buffered_data_source.h
+++ b/webkit/glue/media/buffered_data_source.h
@@ -137,9 +137,6 @@ class BufferedDataSource : public WebDataSource {
   // A webframe for loading.
   WebKit::WebFrame* frame_;
 
-  // True if the media resource has a single origin.
-  bool single_origin_;
-
   // A resource loader for the media resource.
   scoped_refptr<BufferedResourceLoader> loader_;
 
diff --git a/webkit/glue/media/buffered_resource_loader.cc b/webkit/glue/media/buffered_resource_loader.cc
index b33a4e4..a86d2dd 100644
--- a/webkit/glue/media/buffered_resource_loader.cc
+++ b/webkit/glue/media/buffered_resource_loader.cc
@@ -61,6 +61,7 @@ BufferedResourceLoader::BufferedResourceLoader(
       url_(url),
       first_byte_position_(first_byte_position),
       last_byte_position_(last_byte_position),
+      single_origin_(true),
       start_callback_(NULL),
       offset_(0),
       content_length_(kPositionNotSpecified),
@@ -255,6 +256,10 @@ void BufferedResourceLoader::willSendRequest(
     return;
   }
 
+  // Only allow |single_origin_| if we haven't seen a different origin yet.
+  if (single_origin_)
+    single_origin_ = url_.GetOrigin() == GURL(newRequest.url()).GetOrigin();
+
   url_ = newRequest.url();
 }
 
@@ -425,6 +430,10 @@ void BufferedResourceLoader::didFail(
   Release();
 }
 
+bool BufferedResourceLoader::HasSingleOrigin() const {
+  return single_origin_;
+}
+
 /////////////////////////////////////////////////////////////////////////////
 // Helper methods.
 void BufferedResourceLoader::EnableDeferIfNeeded() {
diff --git a/webkit/glue/media/buffered_resource_loader.h b/webkit/glue/media/buffered_resource_loader.h
index 80e9632..fe01fb4 100644
--- a/webkit/glue/media/buffered_resource_loader.h
+++ b/webkit/glue/media/buffered_resource_loader.h
@@ -139,6 +139,8 @@ class BufferedResourceLoader :
       WebKit::WebURLLoader* loader,
       const WebKit::WebURLError&);
 
+  bool HasSingleOrigin() const;
+
  protected:
   friend class base::RefCountedThreadSafe<BufferedResourceLoader>;
 
@@ -212,6 +214,7 @@ class BufferedResourceLoader :
   GURL url_;
   int64 first_byte_position_;
   int64 last_byte_position_;
+  bool single_origin_;
 
   // Callback method that listens to network events.
   scoped_ptr<NetworkEventCallback> event_callback_;
diff --git a/webkit/glue/media/buffered_resource_loader_unittest.cc b/webkit/glue/media/buffered_resource_loader_unittest.cc
index 4ba8a27..dc67edf 100644
--- a/webkit/glue/media/buffered_resource_loader_unittest.cc
+++ b/webkit/glue/media/buffered_resource_loader_unittest.cc
@@ -41,6 +41,11 @@ using WebKit::WebView;
 namespace {
 
 const char* kHttpUrl = "http://test";
+const char kHttpRedirectToSameDomainUrl1[] = "http://test/ing";
+const char kHttpRedirectToSameDomainUrl2[] = "http://test/ing2";
+const char kHttpRedirectToDifferentDomainUrl1[] = "http://test2";
+const char kHttpRedirectToDifferentDomainUrl2[] = "http://test2/ing";
+
 const int kDataSize = 1024;
 const int kHttpOK = 200;
 const int kHttpPartialContent = 206;
@@ -67,8 +72,6 @@ ACTION_P(RequestCanceled, loader) {
 class BufferedResourceLoaderTest : public testing::Test {
  public:
   BufferedResourceLoaderTest() {
-    url_loader_ = new NiceMock<MockWebURLLoader>();
-
     for (int i = 0; i < kDataSize; ++i)
       data_[i] = i;
   }
@@ -83,6 +86,7 @@ class BufferedResourceLoaderTest : public testing::Test {
 
     frame_.reset(new NiceMock<MockWebFrame>());
 
+    url_loader_ = new NiceMock<MockWebURLLoader>();
     loader_ = new BufferedResourceLoader(gurl_,
                                          first_position_, last_position_);
     loader_->SetURLLoaderForTest(url_loader_);
@@ -137,11 +141,22 @@ class BufferedResourceLoaderTest : public testing::Test {
     EXPECT_TRUE(loader_->partial_response());
   }
 
+  void Redirect(const char* url) {
+    GURL redirectUrl(url);
+    WebKit::WebURLRequest newRequest(redirectUrl);
+    WebKit::WebURLResponse redirectResponse(gurl_);
+
+    loader_->willSendRequest(url_loader_, newRequest, redirectResponse);
+
+    MessageLoop::current()->RunAllPending();
+  }
+
   void StopWhenLoad() {
     InSequence s;
     EXPECT_CALL(*url_loader_, cancel())
         .WillOnce(RequestCanceled(loader_));
     loader_->Stop();
+    loader_ = NULL;
   }
 
   // Helper method to write to |loader_| from |data_|.
@@ -479,6 +494,64 @@ TEST_F(BufferedResourceLoaderTest, AllowDefer_DeferredReadPastWindow) {
   ReadLoader(20, 5, buffer);
   StopWhenLoad();
 }
+
+// NOTE: This test will need to be reworked a little once
+// http://code.google.com/p/chromium/issues/detail?id=72578
+// is fixed.
+TEST_F(BufferedResourceLoaderTest, HasSingleOrigin) {
+  // Make sure no redirect case works as expected.
+  Initialize(kHttpUrl, -1, -1);
+  Start();
+  FullResponse(1024);
+  EXPECT_TRUE(loader_->HasSingleOrigin());
+  StopWhenLoad();
+
+  // Test redirect to the same domain.
+  Initialize(kHttpUrl, -1, -1);
+  Start();
+  Redirect(kHttpRedirectToSameDomainUrl1);
+  FullResponse(1024);
+  EXPECT_TRUE(loader_->HasSingleOrigin());
+  StopWhenLoad();
+
+  // Test redirect twice to the same domain.
+  Initialize(kHttpUrl, -1, -1);
+  Start();
+  Redirect(kHttpRedirectToSameDomainUrl1);
+  Redirect(kHttpRedirectToSameDomainUrl2);
+  FullResponse(1024);
+  EXPECT_TRUE(loader_->HasSingleOrigin());
+  StopWhenLoad();
+
+  // Test redirect to a different domain.
+  Initialize(kHttpUrl, -1, -1);
+  Start();
+  Redirect(kHttpRedirectToDifferentDomainUrl1);
+  FullResponse(1024);
+  EXPECT_FALSE(loader_->HasSingleOrigin());
+  StopWhenLoad();
+
+  // Test redirect twice to a different domain.
+  Initialize(kHttpUrl, -1, -1);
+  Start();
+  Redirect(kHttpRedirectToDifferentDomainUrl1);
+  Redirect(kHttpRedirectToDifferentDomainUrl2);
+  FullResponse(1024);
+  EXPECT_FALSE(loader_->HasSingleOrigin());
+  StopWhenLoad();
+
+  // Test to a different domain and then back to the same domain.
+  // NOTE: A different origin was encountered at least once so that
+  //       makes HasSingleOrigin() become false.
+  Initialize(kHttpUrl, -1, -1);
+  Start();
+  Redirect(kHttpRedirectToDifferentDomainUrl1);
+  Redirect(kHttpRedirectToSameDomainUrl1);
+  FullResponse(1024);
+  EXPECT_FALSE(loader_->HasSingleOrigin());
+  StopWhenLoad();
+}
+
 // TODO(hclam): add unit test for defer loading.
 
 }  // namespace webkit_glue
diff --git a/webkit/glue/media/simple_data_source.cc b/webkit/glue/media/simple_data_source.cc
index f4f92dd..07f75878 100644
--- a/webkit/glue/media/simple_data_source.cc
+++ b/webkit/glue/media/simple_data_source.cc
@@ -113,7 +113,10 @@ void SimpleDataSource::willSendRequest(
     WebKit::WebURLRequest& newRequest,
     const WebKit::WebURLResponse& redirectResponse) {
   DCHECK(MessageLoop::current() == render_loop_);
-  single_origin_ = url_.GetOrigin() == GURL(newRequest.url()).GetOrigin();
+
+  // Only allow |single_origin_| if we haven't seen a different origin yet.
+  if (single_origin_)
+    single_origin_ = url_.GetOrigin() == GURL(newRequest.url()).GetOrigin();
 
   url_ = newRequest.url();
 }
diff --git a/webkit/glue/media/simple_data_source_unittest.cc b/webkit/glue/media/simple_data_source_unittest.cc
index 48131b1..a9a70a4 100644
--- a/webkit/glue/media/simple_data_source_unittest.cc
+++ b/webkit/glue/media/simple_data_source_unittest.cc
@@ -42,6 +42,10 @@ const char kDataUrl[] =
     "data:text/plain;base64,YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXoK";
 const char kDataUrlDecoded[] = "abcdefghijklmnopqrstuvwxyz";
 const char kInvalidUrl[] = "whatever://test";
+const char kHttpRedirectToSameDomainUrl1[] = "http://test/ing";
+const char kHttpRedirectToSameDomainUrl2[] = "http://test/ing2";
+const char kHttpRedirectToDifferentDomainUrl1[] = "http://test2";
+const char kHttpRedirectToDifferentDomainUrl2[] = "http://test2/ing";
 
 }  // namespace
 
@@ -113,6 +117,16 @@ class SimpleDataSourceTest : public testing::Test {
     MessageLoop::current()->RunAllPending();
   }
 
+  void Redirect(const char* url) {
+    GURL redirectUrl(url);
+    WebKit::WebURLRequest newRequest(redirectUrl);
+    WebKit::WebURLResponse redirectResponse(gurl_);
+
+    data_source_->willSendRequest(url_loader_, newRequest, redirectResponse);
+
+    MessageLoop::current()->RunAllPending();
+  }
+
   void DestroyDataSource() {
     data_source_->Stop(media::NewExpectedCallback());
     MessageLoop::current()->RunAllPending();
@@ -213,4 +227,55 @@ TEST_F(SimpleDataSourceTest, AsyncRead) {
   DestroyDataSource();
 }
 
+// NOTE: This test will need to be reworked a little once
+// http://code.google.com/p/chromium/issues/detail?id=72578
+// is fixed.
+TEST_F(SimpleDataSourceTest, HasSingleOrigin) {
+  // Make sure no redirect case works as expected.
+  InitializeDataSource(kHttpUrl, media::NewExpectedCallback());
+  RequestSucceeded(false);
+  EXPECT_TRUE(data_source_->HasSingleOrigin());
+  DestroyDataSource();
+
+  // Test redirect to the same domain.
+  InitializeDataSource(kHttpUrl, media::NewExpectedCallback());
+  Redirect(kHttpRedirectToSameDomainUrl1);
+  RequestSucceeded(false);
+  EXPECT_TRUE(data_source_->HasSingleOrigin());
+  DestroyDataSource();
+
+  // Test redirect twice to the same domain.
+  InitializeDataSource(kHttpUrl, media::NewExpectedCallback());
+  Redirect(kHttpRedirectToSameDomainUrl1);
+  Redirect(kHttpRedirectToSameDomainUrl2);
+  RequestSucceeded(false);
+  EXPECT_TRUE(data_source_->HasSingleOrigin());
+  DestroyDataSource();
+
+  // Test redirect to a different domain.
+  InitializeDataSource(kHttpUrl, media::NewExpectedCallback());
+  Redirect(kHttpRedirectToDifferentDomainUrl1);
+  RequestSucceeded(false);
+  EXPECT_FALSE(data_source_->HasSingleOrigin());
+  DestroyDataSource();
+
+  // Test redirect twice to a different domain.
+  InitializeDataSource(kHttpUrl, media::NewExpectedCallback());
+  Redirect(kHttpRedirectToDifferentDomainUrl1);
+  Redirect(kHttpRedirectToDifferentDomainUrl2);
+  RequestSucceeded(false);
+  EXPECT_FALSE(data_source_->HasSingleOrigin());
+  DestroyDataSource();
+
+  // Test to a different domain and then back to the same domain.
+  // NOTE: A different origin was encountered at least once so that
+  //       makes HasSingleOrigin() become false.
+  InitializeDataSource(kHttpUrl, media::NewExpectedCallback());
+  Redirect(kHttpRedirectToDifferentDomainUrl1);
+  Redirect(kHttpRedirectToSameDomainUrl1);
+  RequestSucceeded(false);
+  EXPECT_FALSE(data_source_->HasSingleOrigin());
+  DestroyDataSource();
+}
+
 }  // namespace webkit_glue