Load manifest data for newest cache from storage to compare against fetched manifest.

TEST=updated
BUG=none

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

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

3 files changed, 102 insertions, 65 deletions

diff --git a/webkit/appcache/appcache_update_job.cc b/webkit/appcache/appcache_update_job.cc
index 7887ee1..efcd065 100644
--- a/webkit/appcache/appcache_update_job.cc
+++ b/webkit/appcache/appcache_update_job.cc
@@ -111,7 +111,9 @@ AppCacheUpdateJob::AppCacheUpdateJob(AppCacheService* service,
       ALLOW_THIS_IN_INITIALIZER_LIST(manifest_info_write_callback_(
           this, &AppCacheUpdateJob::OnManifestInfoWriteComplete)),
       ALLOW_THIS_IN_INITIALIZER_LIST(manifest_data_write_callback_(
-          this, &AppCacheUpdateJob::OnManifestDataWriteComplete)) {
+          this, &AppCacheUpdateJob::OnManifestDataWriteComplete)),
+      ALLOW_THIS_IN_INITIALIZER_LIST(manifest_data_read_callback_(
+          this, &AppCacheUpdateJob::OnManifestDataReadComplete)) {
   DCHECK(group_);
   manifest_url_ = group_->manifest_url();
 }
@@ -637,22 +639,30 @@ void AppCacheUpdateJob::NotifyAllAssociatedHosts(EventID event_id) {
 
 void AppCacheUpdateJob::CheckIfManifestChanged() {
   DCHECK(update_type_ == UPGRADE_ATTEMPT);
-  /*
   AppCacheEntry* entry =
       group_->newest_complete_cache()->GetEntry(manifest_url_);
-  */
-  // TODO(jennb): load manifest data from entry (async), continues in callback
-  // callback invokes ContinueCheckIfManifestChanged
-  // For now, schedule a task to continue checking with fake loaded data
-  MessageLoop::current()->PostTask(FROM_HERE,
-      method_factory_.NewRunnableMethod(
-          &AppCacheUpdateJob::ContinueCheckIfManifestChanged,
-          simulate_manifest_changed_ ? "different" : manifest_data_));
+  DCHECK(entry);
+
+  // Load manifest data from storage to compare against fetched manifest.
+  manifest_response_reader_.reset(
+      service_->storage()->CreateResponseReader(manifest_url_,
+                                                entry->response_id()));
+  read_manifest_buffer_ = new net::IOBuffer(kBufferSize);
+  manifest_response_reader_->ReadData(read_manifest_buffer_, kBufferSize,
+      &manifest_data_read_callback_);  // async read
 }
 
-void AppCacheUpdateJob::ContinueCheckIfManifestChanged(
-    const std::string& loaded_manifest) {
-  ContinueHandleManifestFetchCompleted(manifest_data_ != loaded_manifest);
+void AppCacheUpdateJob::OnManifestDataReadComplete(int result) {
+  if (result > 0) {
+    loaded_manifest_data_.append(read_manifest_buffer_->data(), result);
+    manifest_response_reader_->ReadData(read_manifest_buffer_, kBufferSize,
+        &manifest_data_read_callback_);  // read more
+  } else {
+    read_manifest_buffer_ = NULL;
+    manifest_response_reader_.reset();
+    ContinueHandleManifestFetchCompleted(
+        result < 0 || manifest_data_ != loaded_manifest_data_);
+  }
 }
 
 void AppCacheUpdateJob::BuildUrlFileList(const Manifest& manifest) {
diff --git a/webkit/appcache/appcache_update_job.h b/webkit/appcache/appcache_update_job.h
index b65ecd5..b60ca0b 100644
--- a/webkit/appcache/appcache_update_job.h
+++ b/webkit/appcache/appcache_update_job.h
@@ -111,12 +111,7 @@ class AppCacheUpdateJob : public URLRequest::Delegate,
   // Checks if manifest is byte for byte identical with the manifest
   // in the newest application cache.
   void CheckIfManifestChanged();
-  void ContinueCheckIfManifestChanged(const std::string& loaded_manifest);
-
-  // TODO(jennb): delete when able to mock storage behavior
-  void SimulateManifestChanged(bool changed) {
-    simulate_manifest_changed_ = changed;
-  }
+  void OnManifestDataReadComplete(int result);
 
   // Creates the list of files that may need to be fetched and initiates
   // fetches. Section 6.9.4 steps 12-17
@@ -194,12 +189,13 @@ class AppCacheUpdateJob : public URLRequest::Delegate,
   std::string manifest_refetch_data_;
   scoped_ptr<net::HttpResponseInfo> manifest_response_info_;
   scoped_ptr<AppCacheResponseWriter> manifest_response_writer_;
+  scoped_refptr<net::IOBuffer> read_manifest_buffer_;
+  std::string loaded_manifest_data_;
+  scoped_ptr<AppCacheResponseReader> manifest_response_reader_;
 
   net::CompletionCallbackImpl<AppCacheUpdateJob> manifest_info_write_callback_;
   net::CompletionCallbackImpl<AppCacheUpdateJob> manifest_data_write_callback_;
-
-  // TODO(jennb): delete when able to mock storage behavior
-  bool simulate_manifest_changed_;
+  net::CompletionCallbackImpl<AppCacheUpdateJob> manifest_data_read_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(AppCacheUpdateJob);
 };
diff --git a/webkit/appcache/appcache_update_job_unittest.cc b/webkit/appcache/appcache_update_job_unittest.cc
index 79ee787..aae7ef8 100644
--- a/webkit/appcache/appcache_update_job_unittest.cc
+++ b/webkit/appcache/appcache_update_job_unittest.cc
@@ -260,8 +260,8 @@ class AppCacheUpdateJobTest : public testing::Test,
       group_ = new AppCacheGroup(service_.get(), GURL("http://failme"));
 
       // Give the group some existing caches.
-      AppCache* cache1 = MakeCacheForGroup(1);
-      AppCache* cache2 = MakeCacheForGroup(2);
+      AppCache* cache1 = MakeCacheForGroup(1, 111);
+      AppCache* cache2 = MakeCacheForGroup(2, 222);
 
       // Associate some hosts with caches in the group.
       MockFrontend mock_frontend1;
@@ -350,7 +350,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(1);
+    AppCache* cache = MakeCacheForGroup(1, 111);
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -438,7 +438,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(1);
+    AppCache* cache = MakeCacheForGroup(1, 111);
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -521,7 +521,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(1);
+    AppCache* cache = MakeCacheForGroup(1, 111);
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -556,7 +556,11 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(1);
+    // Create response writer to get a response id.
+    response_writer_.reset(
+        service_->storage()->CreateResponseWriter(group_->manifest_url()));
+
+    AppCache* cache = MakeCacheForGroup(1, response_writer_->response_id());
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -564,12 +568,6 @@ class AppCacheUpdateJobTest : public testing::Test,
     host1->AssociateCache(cache);
     host2->AssociateCache(cache);
 
-    // TODO(jennb): simulate this by mocking storage behavior instead
-    update->SimulateManifestChanged(false);  // unchanged
-
-    update->StartUpdate(NULL, GURL::EmptyGURL());
-    EXPECT_TRUE(update->manifest_url_request_ != NULL);
-
     // Set up checks for when update job finishes.
     do_checks_after_update_finished_ = true;
     expect_group_obsolete_ = false;
@@ -582,6 +580,34 @@ class AppCacheUpdateJobTest : public testing::Test,
     frontend2->AddExpectedEvent(ids2, CHECKING_EVENT);
     frontend2->AddExpectedEvent(ids2, NO_UPDATE_EVENT);
 
+    // Seed storage with expected manifest data.
+    const std::string seed_data(
+        "CACHE MANIFEST\n"
+        "explicit1\n"
+        "FALLBACK:\n"
+        "fallback1 fallback1a\n"
+        "NETWORK:\n"
+        "*\n");
+    scoped_refptr<net::StringIOBuffer> io_buffer =
+        new net::StringIOBuffer(seed_data);
+    write_callback_.reset(
+        new net::CompletionCallbackImpl<AppCacheUpdateJobTest>(this,
+            &AppCacheUpdateJobTest::StartUpdateAfterSeedingManifestData));
+    response_writer_->WriteData(io_buffer, seed_data.length(),
+                                write_callback_.get());
+
+    // Start update after data write completes asynchronously.
+  }
+
+  void StartUpdateAfterSeedingManifestData(int result) {
+    ASSERT_GT(result, 0);
+    write_callback_.reset();
+    response_writer_.reset();
+
+    AppCacheUpdateJob* update = group_->update_job_;
+    update->StartUpdate(NULL, GURL::EmptyGURL());
+    EXPECT_TRUE(update->manifest_url_request_ != NULL);
+
     WaitForUpdateToFinish();
   }
 
@@ -619,7 +645,12 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId());
+    // Create a response writer to get a response id.
+    response_writer_.reset(
+        service_->storage()->CreateResponseWriter(group_->manifest_url()));
+
+    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId(),
+                                        response_writer_->response_id());
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -627,12 +658,6 @@ class AppCacheUpdateJobTest : public testing::Test,
     host1->AssociateCache(cache);
     host2->AssociateCache(cache);
 
-    // TODO(jennb): simulate this by mocking storage behavior instead
-    update->SimulateManifestChanged(true);  // changed
-
-    update->StartUpdate(NULL, GURL::EmptyGURL());
-    EXPECT_TRUE(update->manifest_url_request_ != NULL);
-
     // Set up checks for when update job finishes.
     do_checks_after_update_finished_ = true;
     expect_group_obsolete_ = false;
@@ -652,7 +677,17 @@ class AppCacheUpdateJobTest : public testing::Test,
     frontend2->AddExpectedEvent(ids2, PROGRESS_EVENT);
     frontend2->AddExpectedEvent(ids2, UPDATE_READY_EVENT);
 
-    WaitForUpdateToFinish();
+    // Seed storage with expected manifest data different from manifest1.
+    const std::string seed_data("different");
+    scoped_refptr<net::StringIOBuffer> io_buffer =
+        new net::StringIOBuffer(seed_data);
+    write_callback_.reset(
+        new net::CompletionCallbackImpl<AppCacheUpdateJobTest>(this,
+            &AppCacheUpdateJobTest::StartUpdateAfterSeedingManifestData));
+    response_writer_->WriteData(io_buffer, seed_data.length(),
+                                write_callback_.get());
+
+    // Start update after data write completes asynchronously.
   }
 
   void UpgradeSuccessMergedTypesTest() {
@@ -664,7 +699,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId());
+    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId(), 42);
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -677,9 +712,6 @@ class AppCacheUpdateJobTest : public testing::Test,
     cache->AddEntry(http_server_->TestServerPage("files/explicit1"),
                     AppCacheEntry(AppCacheEntry::MASTER, 111));
 
-    // TODO(jennb): simulate this by mocking storage behavior instead
-    update->SimulateManifestChanged(true);  // changed
-
     update->StartUpdate(NULL, GURL::EmptyGURL());
     EXPECT_TRUE(update->manifest_url_request_ != NULL);
 
@@ -693,8 +725,9 @@ class AppCacheUpdateJobTest : public testing::Test,
     frontend1->AddExpectedEvent(ids1, CHECKING_EVENT);
     frontend1->AddExpectedEvent(ids1, DOWNLOADING_EVENT);
     frontend1->AddExpectedEvent(ids1, PROGRESS_EVENT);  // explicit1 (load)
+    frontend1->AddExpectedEvent(ids1, PROGRESS_EVENT);  // manifest (load)
     frontend1->AddExpectedEvent(ids1, PROGRESS_EVENT);  // explicit1 (fetch)
-    frontend1->AddExpectedEvent(ids1, PROGRESS_EVENT);  // manifest
+    frontend1->AddExpectedEvent(ids1, PROGRESS_EVENT);  // manifest (fetch)
     frontend1->AddExpectedEvent(ids1, UPDATE_READY_EVENT);
     MockFrontend::HostIds ids2(1, host2->host_id());
     frontend2->AddExpectedEvent(ids2, CHECKING_EVENT);
@@ -702,6 +735,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     frontend2->AddExpectedEvent(ids2, PROGRESS_EVENT);
     frontend2->AddExpectedEvent(ids2, PROGRESS_EVENT);
     frontend2->AddExpectedEvent(ids2, PROGRESS_EVENT);
+    frontend2->AddExpectedEvent(ids2, PROGRESS_EVENT);
     frontend2->AddExpectedEvent(ids2, UPDATE_READY_EVENT);
 
     WaitForUpdateToFinish();
@@ -740,7 +774,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId());
+    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId(), 99);
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -748,9 +782,6 @@ class AppCacheUpdateJobTest : public testing::Test,
     host1->AssociateCache(cache);
     host2->AssociateCache(cache);
 
-    // TODO(jennb): simulate this by mocking storage behavior instead
-    update->SimulateManifestChanged(true);  // changed
-
     update->StartUpdate(NULL, GURL::EmptyGURL());
     EXPECT_TRUE(update->manifest_url_request_ != NULL);
 
@@ -786,7 +817,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId());
+    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId(), 25);
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -805,9 +836,6 @@ class AppCacheUpdateJobTest : public testing::Test,
         http_server_->TestServerPage("files/servererror"),
         AppCacheEntry(AppCacheEntry::MASTER, 444));
 
-    // TODO(jennb): simulate this by mocking storage behavior instead
-    update->SimulateManifestChanged(true);  // changed
-
     update->StartUpdate(NULL, GURL::EmptyGURL());
     EXPECT_TRUE(update->manifest_url_request_ != NULL);
 
@@ -860,7 +888,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId());
+    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId(), 33);
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -868,9 +896,6 @@ class AppCacheUpdateJobTest : public testing::Test,
     host1->AssociateCache(cache);
     host2->AssociateCache(cache);
 
-    // TODO(jennb): simulate this by mocking storage behavior instead
-    update->SimulateManifestChanged(true);  // changed
-
     update->StartUpdate(NULL, GURL::EmptyGURL());
     EXPECT_TRUE(update->manifest_url_request_ != NULL);
 
@@ -1083,7 +1108,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId());
+    AppCache* cache = MakeCacheForGroup(service_->storage()->NewCacheId(), 11);
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -1091,9 +1116,6 @@ class AppCacheUpdateJobTest : public testing::Test,
     host1->AssociateCache(cache);
     host2->AssociateCache(cache);
 
-    // TODO(jennb): simulate this by mocking storage behavior instead
-    update->SimulateManifestChanged(true);  // changed
-
     update->StartUpdate(NULL, GURL::EmptyGURL());
 
     // Set up checks for when update job finishes.
@@ -1158,7 +1180,7 @@ class AppCacheUpdateJobTest : public testing::Test,
     AppCacheUpdateJob* update = new AppCacheUpdateJob(service_.get(), group_);
     group_->update_job_ = update;
 
-    AppCache* cache = MakeCacheForGroup(1);
+    AppCache* cache = MakeCacheForGroup(1, 111);
     MockFrontend* frontend1 = MakeMockFrontend();
     MockFrontend* frontend2 = MakeMockFrontend();
     AppCacheHost* host1 = MakeHost(1, frontend1);
@@ -1229,11 +1251,16 @@ class AppCacheUpdateJobTest : public testing::Test,
     service_->set_request_context(request_context_);
   }
 
-  AppCache* MakeCacheForGroup(int64 cache_id) {
+  AppCache* MakeCacheForGroup(int64 cache_id, int64 manifest_response_id) {
     AppCache* cache = new AppCache(service_.get(), cache_id);
     cache->set_complete(true);
     cache->set_update_time(base::TimeTicks::Now());
     group_->AddCache(cache);
+
+    // Add manifest entry to cache.
+    cache->AddEntry(group_->manifest_url(),
+        AppCacheEntry(AppCacheEntry::MANIFEST, manifest_response_id));
+
     return cache;
   }
 
@@ -1450,6 +1477,10 @@ class AppCacheUpdateJobTest : public testing::Test,
   scoped_refptr<AppCache> protect_newest_cache_;
   scoped_ptr<base::WaitableEvent> event_;
 
+  scoped_ptr<AppCacheResponseWriter> response_writer_;
+  scoped_ptr<net::CompletionCallbackImpl<AppCacheUpdateJobTest> >
+      write_callback_;
+
   // Hosts used by an async test that need to live until update job finishes.
   // Otherwise, test can put host on the stack instead of here.
   std::vector<AppCacheHost*> hosts_;