Make appcache update conditionally include If-Modified-Since and If-None-Match headers when fetching a manifest.

TEST=new tests added
BUG=none

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

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

4 files changed, 410 insertions, 6 deletions

diff --git a/webkit/appcache/appcache_response.cc b/webkit/appcache/appcache_response.cc
index 9ae5b29..09894a2 100644
--- a/webkit/appcache/appcache_response.cc
+++ b/webkit/appcache/appcache_response.cc
@@ -146,6 +146,11 @@ void AppCacheResponseReader::ReadInfo(HttpResponseInfoIOBuffer* info_buf,
   }
 
   int size = entry_->GetDataSize(kResponseInfoIndex);
+  if (size <= 0) {
+    ScheduleIOCompletionCallback(net::ERR_CACHE_MISS);
+    return;
+  }
+
   info_buffer_ = info_buf;
   buffer_ = new net::IOBuffer(size);
   ReadRaw(kResponseInfoIndex, 0, buffer_.get(), size);
diff --git a/webkit/appcache/appcache_update_job.cc b/webkit/appcache/appcache_update_job.cc
index de7b73c..dbd763f8 100644
--- a/webkit/appcache/appcache_update_job.cc
+++ b/webkit/appcache/appcache_update_job.cc
@@ -198,10 +198,58 @@ void AppCacheUpdateJob::FetchManifest(bool is_first_fetch) {
       UpdateJobInfo::MANIFEST_FETCH : UpdateJobInfo::MANIFEST_REFETCH;
   manifest_url_request_->SetUserData(this, new UpdateJobInfo(fetch_type));
   manifest_url_request_->set_context(service_->request_context());
-  // TODO(jennb): add "If-Modified-Since" if have previous date
   manifest_url_request_->set_load_flags(
       manifest_url_request_->load_flags() | net::LOAD_DISABLE_INTERCEPT);
-  manifest_url_request_->Start();
+
+  // Add any necessary Http headers before sending fetch request.
+  if (is_first_fetch) {
+    AppCacheEntry* entry = (update_type_ == UPGRADE_ATTEMPT) ?
+        group_->newest_complete_cache()->GetEntry(manifest_url_) : NULL;
+    if (entry) {
+      // Asynchronously load response info for manifest from newest cache.
+      service_->storage()->LoadResponseInfo(manifest_url_,
+                                            entry->response_id(), this);
+    } else {
+      AddHttpHeadersAndFetch(manifest_url_request_, NULL);
+    }
+  } else {
+    DCHECK(internal_state_ == REFETCH_MANIFEST);
+    DCHECK(manifest_response_info_.get());
+    AddHttpHeadersAndFetch(manifest_url_request_,
+                           manifest_response_info_.get());
+  }
+}
+
+void AppCacheUpdateJob::AddHttpHeadersAndFetch(
+    URLRequest* request, const net::HttpResponseInfo* info) {
+  DCHECK(request);
+  if (info) {
+    std::string extra_headers;
+
+    // Add If-Modified-Since header if response info has Last-Modified header.
+    const std::string last_modified = "Last-Modified";
+    std::string last_modified_value;
+    info->headers->EnumerateHeader(NULL, last_modified, &last_modified_value);
+    if (!last_modified_value.empty()) {
+      extra_headers.append("If-Modified-Since: ");
+      extra_headers.append(last_modified_value);
+    }
+
+    // Add If-None-Match header if resposne info has ETag header.
+    const std::string etag = "ETag";
+    std::string etag_value;
+    info->headers->EnumerateHeader(NULL, etag, &etag_value);
+    if (!etag_value.empty()) {
+      if (!extra_headers.empty())
+        extra_headers.append("\r\n");
+      extra_headers.append("If-None-Match: ");
+      extra_headers.append(etag_value);
+    }
+
+    if (!extra_headers.empty())
+      request->SetExtraRequestHeaders(extra_headers);
+  }
+  request->Start();
 }
 
 void AppCacheUpdateJob::OnResponseStarted(URLRequest *request) {
@@ -1018,17 +1066,24 @@ bool AppCacheUpdateJob::MaybeLoadFromNewestCache(const GURL& url,
 
 void AppCacheUpdateJob::OnResponseInfoLoaded(
     AppCacheResponseInfo* response_info, int64 response_id) {
+  const net::HttpResponseInfo* http_info = response_info ?
+      response_info->http_response_info() : NULL;
+
+  // Needed response info for a manifest fetch request.
+  if (internal_state_ == FETCH_MANIFEST) {
+    AddHttpHeadersAndFetch(manifest_url_request_, http_info);
+    return;
+  }
+
   LoadingResponses::iterator found = loading_responses_.find(response_id);
   DCHECK(found != loading_responses_.end());
   const GURL& url = found->second;
 
-  if (!response_info) {
+  if (!http_info) {
     LoadFromNewestCacheFailed(url);  // no response found
   } else {
     // Check if response can be re-used according to HTTP caching semantics.
     // Responses with a "vary" header get treated as expired.
-    const net::HttpResponseInfo* http_info =
-        response_info->http_response_info();
     const std::string name = "vary";
     std::string value;
     void* iter = NULL;
diff --git a/webkit/appcache/appcache_update_job.h b/webkit/appcache/appcache_update_job.h
index 840840d..0d343f0 100644
--- a/webkit/appcache/appcache_update_job.h
+++ b/webkit/appcache/appcache_update_job.h
@@ -91,6 +91,11 @@ class AppCacheUpdateJob : public URLRequest::Delegate,
 
   void FetchManifest(bool is_first_fetch);
 
+  // Add extra HTTP headers to the request based on the response info and
+  // start the URL request.
+  void AddHttpHeadersAndFetch(URLRequest* request,
+                              const net::HttpResponseInfo* info);
+
   void OnResponseCompleted(URLRequest* request);
 
   // Retries a 503 request with retry-after header of 0.
diff --git a/webkit/appcache/appcache_update_job_unittest.cc b/webkit/appcache/appcache_update_job_unittest.cc
index 72698e2..45898e8 100644
--- a/webkit/appcache/appcache_update_job_unittest.cc
+++ b/webkit/appcache/appcache_update_job_unittest.cc
@@ -131,7 +131,7 @@ class RetryRequestTestJob : public URLRequestTestJob {
   }
 
   static URLRequestJob* RetryFactory(URLRequest* request,
-                                   const std::string& scheme) {
+                                     const std::string& scheme) {
     ++num_requests_;
     if (num_retries_ > 0 && request->original_url() == kRetryUrl) {
       --num_retries_;
@@ -206,6 +206,72 @@ int RetryRequestTestJob::num_retries_;
 RetryRequestTestJob::RetryHeader RetryRequestTestJob::retry_after_;
 int RetryRequestTestJob::expected_requests_ = 0;
 
+// Helper class to check for certain HTTP headers.
+class HttpHeadersRequestTestJob : public URLRequestTestJob {
+ public:
+  // Call this at the start of each HTTP header-related test.
+  static void Initialize(const std::string& expect_if_modified_since,
+                         const std::string& expect_if_none_match) {
+    expect_if_modified_since_ = expect_if_modified_since;
+    expect_if_none_match_ = expect_if_none_match;
+  }
+
+  // Verifies results at end of test and resets class.
+  static void Verify() {
+    if (!expect_if_modified_since_.empty())
+      EXPECT_TRUE(saw_if_modified_since_);
+    if (!expect_if_none_match_.empty())
+      EXPECT_TRUE(saw_if_none_match_);
+
+    // Reset.
+    expect_if_modified_since_.clear();
+    saw_if_modified_since_ = false;
+    expect_if_none_match_.clear();
+    saw_if_none_match_ = false;
+    already_checked_ = false;
+  }
+
+  static URLRequestJob* IfModifiedSinceFactory(URLRequest* request,
+                                               const std::string& scheme) {
+    if (!already_checked_) {
+      already_checked_ = true;  // only check once for a test
+      const std::string& extra_headers = request->extra_request_headers();
+      const std::string if_modified_since = "If-Modified-Since: ";
+      size_t pos = extra_headers.find(if_modified_since);
+      if (pos != std::string::npos) {
+        saw_if_modified_since_ = (0 == extra_headers.compare(
+            pos + if_modified_since.length(),
+            expect_if_modified_since_.length(),
+            expect_if_modified_since_));
+      }
+
+      const std::string if_none_match = "If-None-Match: ";
+      pos = extra_headers.find(if_none_match);
+      if (pos != std::string::npos) {
+        saw_if_none_match_ = (0 == extra_headers.compare(
+            pos + if_none_match.length(),
+            expect_if_none_match_.length(),
+            expect_if_none_match_));
+      }
+    }
+    return NULL;
+  }
+
+ private:
+  static std::string expect_if_modified_since_;
+  static bool saw_if_modified_since_;
+  static std::string expect_if_none_match_;
+  static bool saw_if_none_match_;
+  static bool already_checked_;
+};
+
+// static
+std::string HttpHeadersRequestTestJob::expect_if_modified_since_;
+bool HttpHeadersRequestTestJob::saw_if_modified_since_ = false;
+std::string HttpHeadersRequestTestJob::expect_if_none_match_;
+bool HttpHeadersRequestTestJob::saw_if_none_match_ = false;
+bool HttpHeadersRequestTestJob::already_checked_ = false;
+
 class AppCacheUpdateJobTest : public testing::Test,
                               public AppCacheGroup::UpdateObserver {
  public:
@@ -2116,6 +2182,258 @@ class AppCacheUpdateJobTest : public testing::Test,
     group_->AddUpdateObserver(this);
   }
 
+  void IfModifiedSinceTest() {
+    ASSERT_EQ(MessageLoop::TYPE_IO, MessageLoop::current()->type());
+
+    old_factory_ = URLRequest::RegisterProtocolFactory(
+        "http", HttpHeadersRequestTestJob::IfModifiedSinceFactory);
+    registered_factory_ = true;
+
+    MakeService();
+    group_ = new AppCacheGroup(service_.get(), GURL("http://headertest"), 111);
+    AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
+    group_->update_job_ = update;
+
+    // First test against a cache attempt. Will start manifest fetch
+    // synchronously.
+    HttpHeadersRequestTestJob::Initialize("", "");
+    MockFrontend mock_frontend;
+    AppCacheHost host(1, &mock_frontend, service_.get());
+    update->StartUpdate(&host, GURL::EmptyGURL());
+    HttpHeadersRequestTestJob::Verify();
+    delete update;
+
+    // Now simulate a refetch manifest request. Will start fetch request
+    // synchronously.
+    const char data[] =
+        "HTTP/1.1 200 OK\0"
+        "\0";
+    net::HttpResponseHeaders* headers =
+        new net::HttpResponseHeaders(std::string(data, arraysize(data)));
+    net::HttpResponseInfo* response_info = new net::HttpResponseInfo();
+    response_info->headers = headers;  // adds ref to headers
+
+    HttpHeadersRequestTestJob::Initialize("", "");
+    update = new AppCacheUpdateJob(service_.get(), group_);
+    group_->update_job_ = update;
+    group_->update_status_ = AppCacheGroup::DOWNLOADING;
+    update->manifest_response_info_.reset(response_info);
+    update->internal_state_ = AppCacheUpdateJob::REFETCH_MANIFEST;
+    update->FetchManifest(false);  // not first request
+    HttpHeadersRequestTestJob::Verify();
+    delete update;
+
+    // Change the headers to include a Last-Modified header. Manifest refetch
+    // should include If-Modified-Since header.
+    const char data2[] =
+        "HTTP/1.1 200 OK\0"
+        "Last-Modified: Sat, 29 Oct 1994 19:43:31 GMT\0"
+        "\0";
+    net::HttpResponseHeaders* headers2 =
+        new net::HttpResponseHeaders(std::string(data2, arraysize(data2)));
+    response_info = new net::HttpResponseInfo();
+    response_info->headers = headers2;
+
+    HttpHeadersRequestTestJob::Initialize("Sat, 29 Oct 1994 19:43:31 GMT", "");
+    update = new AppCacheUpdateJob(service_.get(), group_);
+    group_->update_job_ = update;
+    group_->update_status_ = AppCacheGroup::DOWNLOADING;
+    update->manifest_response_info_.reset(response_info);
+    update->internal_state_ = AppCacheUpdateJob::REFETCH_MANIFEST;
+    update->FetchManifest(false);  // not first request
+    HttpHeadersRequestTestJob::Verify();
+    delete update;
+
+    UpdateFinished();
+  }
+
+  void IfModifiedSinceUpgradeTest() {
+    ASSERT_EQ(MessageLoop::TYPE_IO, MessageLoop::current()->type());
+
+    HttpHeadersRequestTestJob::Initialize("Sat, 29 Oct 1994 19:43:31 GMT", "");
+    old_factory_ = URLRequest::RegisterProtocolFactory(
+        "http", HttpHeadersRequestTestJob::IfModifiedSinceFactory);
+    registered_factory_ = true;
+
+    MakeService();
+    group_ = new AppCacheGroup(
+        service_.get(), http_server_->TestServerPage("files/manifest1"), 111);
+    AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
+    group_->update_job_ = update;
+
+    // Give the newest cache a manifest enry that is in storage.
+    response_writer_.reset(
+        service_->storage()->CreateResponseWriter(group_->manifest_url()));
+
+    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId(),
+                                        response_writer_->response_id());
+    MockFrontend* frontend = MakeMockFrontend();
+    AppCacheHost* host = MakeHost(1, frontend);
+    host->AssociateCache(cache);
+
+    // Set up checks for when update job finishes.
+    do_checks_after_update_finished_ = true;
+    expect_group_obsolete_ = false;
+    expect_group_has_cache_ = true;
+    expect_old_cache_ = cache;
+    tested_manifest_ = MANIFEST1;
+    MockFrontend::HostIds ids1(1, host->host_id());
+    frontend->AddExpectedEvent(ids1, CHECKING_EVENT);
+    frontend->AddExpectedEvent(ids1, DOWNLOADING_EVENT);
+    frontend->AddExpectedEvent(ids1, PROGRESS_EVENT);
+    frontend->AddExpectedEvent(ids1, PROGRESS_EVENT);
+    frontend->AddExpectedEvent(ids1, UPDATE_READY_EVENT);
+
+    // Seed storage with expected manifest response info that will cause
+    // an If-Modified-Since header to be put in the manifest fetch request.
+    const char data[] =
+        "HTTP/1.1 200 OK\0"
+        "Last-Modified: Sat, 29 Oct 1994 19:43:31 GMT\0"
+        "\0";
+    net::HttpResponseHeaders* headers =
+        new net::HttpResponseHeaders(std::string(data, arraysize(data)));
+    net::HttpResponseInfo* response_info = new net::HttpResponseInfo();
+    response_info->headers = headers;  // adds ref to headers
+    scoped_refptr<HttpResponseInfoIOBuffer> io_buffer =
+        new HttpResponseInfoIOBuffer(response_info);  // adds ref to info
+    write_callback_.reset(
+        new net::CompletionCallbackImpl<AppCacheUpdateJobTest>(this,
+            &AppCacheUpdateJobTest::StartUpdateAfterSeedingStorageData));
+    response_writer_->WriteInfo(io_buffer, write_callback_.get());
+
+    // Start update after data write completes asynchronously.
+  }
+
+  void IfNoneMatchUpgradeTest() {
+    ASSERT_EQ(MessageLoop::TYPE_IO, MessageLoop::current()->type());
+
+    HttpHeadersRequestTestJob::Initialize("", "\"LadeDade\"");
+    old_factory_ = URLRequest::RegisterProtocolFactory(
+        "http", HttpHeadersRequestTestJob::IfModifiedSinceFactory);
+    registered_factory_ = true;
+
+    MakeService();
+    group_ = new AppCacheGroup(
+        service_.get(), http_server_->TestServerPage("files/manifest1"), 111);
+    AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
+    group_->update_job_ = update;
+
+    // Give the newest cache a manifest enry that is in storage.
+    response_writer_.reset(
+        service_->storage()->CreateResponseWriter(group_->manifest_url()));
+
+    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId(),
+                                        response_writer_->response_id());
+    MockFrontend* frontend = MakeMockFrontend();
+    AppCacheHost* host = MakeHost(1, frontend);
+    host->AssociateCache(cache);
+
+    // Set up checks for when update job finishes.
+    do_checks_after_update_finished_ = true;
+    expect_group_obsolete_ = false;
+    expect_group_has_cache_ = true;
+    expect_old_cache_ = cache;
+    tested_manifest_ = MANIFEST1;
+    MockFrontend::HostIds ids1(1, host->host_id());
+    frontend->AddExpectedEvent(ids1, CHECKING_EVENT);
+    frontend->AddExpectedEvent(ids1, DOWNLOADING_EVENT);
+    frontend->AddExpectedEvent(ids1, PROGRESS_EVENT);
+    frontend->AddExpectedEvent(ids1, PROGRESS_EVENT);
+    frontend->AddExpectedEvent(ids1, UPDATE_READY_EVENT);
+
+    // Seed storage with expected manifest response info that will cause
+    // an If-None-Match header to be put in the manifest fetch request.
+    const char data[] =
+        "HTTP/1.1 200 OK\0"
+        "ETag: \"LadeDade\"\0"
+        "\0";
+    net::HttpResponseHeaders* headers =
+        new net::HttpResponseHeaders(std::string(data, arraysize(data)));
+    net::HttpResponseInfo* response_info = new net::HttpResponseInfo();
+    response_info->headers = headers;  // adds ref to headers
+    scoped_refptr<HttpResponseInfoIOBuffer> io_buffer =
+        new HttpResponseInfoIOBuffer(response_info);  // adds ref to info
+    write_callback_.reset(
+        new net::CompletionCallbackImpl<AppCacheUpdateJobTest>(this,
+            &AppCacheUpdateJobTest::StartUpdateAfterSeedingStorageData));
+    response_writer_->WriteInfo(io_buffer, write_callback_.get());
+
+    // Start update after data write completes asynchronously.
+  }
+
+  void IfNoneMatchRefetchTest() {
+    ASSERT_EQ(MessageLoop::TYPE_IO, MessageLoop::current()->type());
+
+    HttpHeadersRequestTestJob::Initialize("", "\"LadeDade\"");
+    old_factory_ = URLRequest::RegisterProtocolFactory(
+        "http", HttpHeadersRequestTestJob::IfModifiedSinceFactory);
+    registered_factory_ = true;
+
+    MakeService();
+    group_ = new AppCacheGroup(service_.get(), GURL("http://headertest"), 111);
+    AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
+    group_->update_job_ = update;
+
+    // Simulate a refetch manifest request that uses an ETag header.
+    const char data[] =
+        "HTTP/1.1 200 OK\0"
+        "ETag: \"LadeDade\"\0"
+        "\0";
+    net::HttpResponseHeaders* headers =
+        new net::HttpResponseHeaders(std::string(data, arraysize(data)));
+    net::HttpResponseInfo* response_info = new net::HttpResponseInfo();
+    response_info->headers = headers;  // adds ref to headers
+
+    update = new AppCacheUpdateJob(service_.get(), group_);
+    group_->update_job_ = update;
+    group_->update_status_ = AppCacheGroup::DOWNLOADING;
+    update->manifest_response_info_.reset(response_info);
+    update->internal_state_ = AppCacheUpdateJob::REFETCH_MANIFEST;
+    update->FetchManifest(false);  // not first request
+    HttpHeadersRequestTestJob::Verify();
+    delete update;
+
+    UpdateFinished();
+  }
+
+  void MultipleHeadersRefetchTest() {
+    ASSERT_EQ(MessageLoop::TYPE_IO, MessageLoop::current()->type());
+
+    // Verify that code is correct when building multiple extra headers.
+    HttpHeadersRequestTestJob::Initialize(
+        "Sat, 29 Oct 1994 19:43:31 GMT", "\"LadeDade\"");
+    old_factory_ = URLRequest::RegisterProtocolFactory(
+        "http", HttpHeadersRequestTestJob::IfModifiedSinceFactory);
+    registered_factory_ = true;
+
+    MakeService();
+    group_ = new AppCacheGroup(service_.get(), GURL("http://headertest"), 111);
+    AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
+    group_->update_job_ = update;
+
+    // Simulate a refetch manifest request that uses an ETag header.
+    const char data[] =
+        "HTTP/1.1 200 OK\0"
+        "Last-Modified: Sat, 29 Oct 1994 19:43:31 GMT\0"
+        "ETag: \"LadeDade\"\0"
+        "\0";
+    net::HttpResponseHeaders* headers =
+        new net::HttpResponseHeaders(std::string(data, arraysize(data)));
+    net::HttpResponseInfo* response_info = new net::HttpResponseInfo();
+    response_info->headers = headers;  // adds ref to headers
+
+    update = new AppCacheUpdateJob(service_.get(), group_);
+    group_->update_job_ = update;
+    group_->update_status_ = AppCacheGroup::DOWNLOADING;
+    update->manifest_response_info_.reset(response_info);
+    update->internal_state_ = AppCacheUpdateJob::REFETCH_MANIFEST;
+    update->FetchManifest(false);  // not first request
+    HttpHeadersRequestTestJob::Verify();
+    delete update;
+
+    UpdateFinished();
+  }
+
   void WaitForUpdateToFinish() {
     if (group_->update_status() == AppCacheGroup::IDLE)
       UpdateFinished();
@@ -2190,6 +2508,7 @@ class AppCacheUpdateJobTest : public testing::Test,
   // has finished. Cannot verify update job internals as update is deleted.
   void VerifyExpectations() {
     RetryRequestTestJob::Verify();
+    HttpHeadersRequestTestJob::Verify();
 
     EXPECT_EQ(expect_group_obsolete_, group_->is_obsolete());
 
@@ -2713,4 +3032,24 @@ TEST_F(AppCacheUpdateJobTest, QueueMasterEntry) {
   RunTestOnIOThread(&AppCacheUpdateJobTest::QueueMasterEntryTest);
 }
 
+TEST_F(AppCacheUpdateJobTest, IfModifiedSince) {
+  RunTestOnIOThread(&AppCacheUpdateJobTest::IfModifiedSinceTest);
+}
+
+TEST_F(AppCacheUpdateJobTest, IfModifiedSinceUpgrade) {
+  RunTestOnIOThread(&AppCacheUpdateJobTest::IfModifiedSinceUpgradeTest);
+}
+
+TEST_F(AppCacheUpdateJobTest, IfNoneMatchUpgrade) {
+  RunTestOnIOThread(&AppCacheUpdateJobTest::IfNoneMatchUpgradeTest);
+}
+
+TEST_F(AppCacheUpdateJobTest, IfNoneMatchRefetch) {
+  RunTestOnIOThread(&AppCacheUpdateJobTest::IfNoneMatchRefetchTest);
+}
+
+TEST_F(AppCacheUpdateJobTest, MultipleHeadersRefetch) {
+  RunTestOnIOThread(&AppCacheUpdateJobTest::MultipleHeadersRefetchTest);
+}
+
 }  // namespace appcache