Re-enabling disabled appcache update unittests. Change comparison of cache update time. Change manifest-fb-404 entry ordering so all 3 requests always happen regardless of which order the url requests complete in.

TEST=none
BUG=none

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

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

5 files changed, 55 insertions, 25 deletions

diff --git a/webkit/appcache/appcache.h b/webkit/appcache/appcache.h
index 08fdd1b..c80cafd 100644
--- a/webkit/appcache/appcache.h
+++ b/webkit/appcache/appcache.h
@@ -60,7 +60,14 @@ class AppCache : public base::RefCounted<AppCache> {
   }
 
   bool IsNewerThan(AppCache* cache) const {
-    return update_time_ > cache->update_time_;
+    if (update_time_ > cache->update_time_)
+      return true;
+
+    // Tie breaker. Newer caches have a larger cache ID.
+    if (update_time_ == cache->update_time_)
+      return cache_id_ > cache->cache_id_;
+
+    return false;
   }
 
   void set_update_time(base::TimeTicks ticks) {
diff --git a/webkit/appcache/appcache_group_unittest.cc b/webkit/appcache/appcache_group_unittest.cc
index 74846af..6eeb209 100644
--- a/webkit/appcache/appcache_group_unittest.cc
+++ b/webkit/appcache/appcache_group_unittest.cc
@@ -75,18 +75,38 @@ TEST(AppCacheGroupTest, AddRemoveCache) {
   group->AddCache(cache3);
   EXPECT_EQ(cache3, group->newest_complete_cache());
 
+  // Adding cache with same update time uses one with larger ID.
+  AppCache* cache4 = new AppCache(&service, 444);
+  cache4->set_complete(true);
+  cache4->set_update_time(ticks + base::TimeDelta::FromDays(1)); // same as 3
+  cache4->set_owning_group(group);
+  group->AddCache(cache4);
+  EXPECT_EQ(cache4, group->newest_complete_cache());
+
+  AppCache* cache5 = new AppCache(&service, 55);  // smaller id
+  cache5->set_complete(true);
+  cache5->set_update_time(ticks + base::TimeDelta::FromDays(1)); // same as 4
+  cache5->set_owning_group(group);
+  group->AddCache(cache5);
+  EXPECT_EQ(cache4, group->newest_complete_cache());  // no change
+
   // Old caches can always be removed.
   EXPECT_TRUE(group->RemoveCache(cache1));
-  EXPECT_EQ(cache3, group->newest_complete_cache());  // newest unchanged
+  EXPECT_EQ(cache4, group->newest_complete_cache());  // newest unchanged
 
   // Cannot remove newest cache if there are older caches.
-  EXPECT_FALSE(group->RemoveCache(cache3));
-  EXPECT_EQ(cache3, group->newest_complete_cache());  // newest unchanged
+  EXPECT_FALSE(group->RemoveCache(cache4));
+  EXPECT_EQ(cache4, group->newest_complete_cache());  // newest unchanged
 
   // Can remove newest cache after all older caches are removed.
   EXPECT_TRUE(group->RemoveCache(cache2));
-  EXPECT_EQ(cache3, group->newest_complete_cache());  // newest unchanged
+  EXPECT_EQ(cache4, group->newest_complete_cache());  // newest unchanged
   EXPECT_TRUE(group->RemoveCache(cache3));
+  EXPECT_EQ(cache4, group->newest_complete_cache());  // newest unchanged
+  EXPECT_TRUE(group->RemoveCache(cache5));
+  EXPECT_EQ(cache4, group->newest_complete_cache());  // newest unchanged
+  EXPECT_TRUE(group->RemoveCache(cache4));            // newest removed
+  EXPECT_FALSE(group->newest_complete_cache());       // no more newest cache
 }
 
 TEST(AppCacheGroupTest, CleanupUnusedGroup) {
diff --git a/webkit/appcache/appcache_update_job.cc b/webkit/appcache/appcache_update_job.cc
index a22b561..146c8f2 100644
--- a/webkit/appcache/appcache_update_job.cc
+++ b/webkit/appcache/appcache_update_job.cc
@@ -337,7 +337,7 @@ void AppCacheUpdateJob::HandleUrlFetchCompleted(URLRequest* request) {
     // whose value doesn't match the manifest url of the application cache
     // being processed, mark the entry as being foreign.
   } else {
-    LOG(INFO) << "Request status: " << request->status().status()
+    LOG(ERROR) << "Request status: " << request->status().status()
         << " os_error: " << request->status().os_error()
         << " response code: " << response_code;
 
@@ -403,7 +403,7 @@ void AppCacheUpdateJob::HandleManifestRefetchCompleted(URLRequest* request) {
     DeleteSoon();
     // TODO(jennb): end of part that needs to be made async.
   } else {
-    LOG(INFO) << "Request status: " << request->status().status()
+    LOG(ERROR) << "Request status: " << request->status().status()
         << " os_error: " << request->status().os_error()
         << " response code: " << response_code;
     ScheduleUpdateRetry(kRerunDelayMs);
diff --git a/webkit/appcache/appcache_update_job_unittest.cc b/webkit/appcache/appcache_update_job_unittest.cc
index e46de0c..f08625d 100644
--- a/webkit/appcache/appcache_update_job_unittest.cc
+++ b/webkit/appcache/appcache_update_job_unittest.cc
@@ -859,20 +859,6 @@ class AppCacheUpdateJobTest : public testing::Test,
       }
       if (expect_newest_cache_)
         EXPECT_EQ(expect_newest_cache_, group_->newest_complete_cache());
-      switch (tested_manifest_) {
-        case MANIFEST1:
-          VerifyManifest1(group_->newest_complete_cache());
-          break;
-        case MANIFEST_MERGED_TYPES:
-          VerifyManifestMergedTypes(group_->newest_complete_cache());
-          break;
-        case EMPTY_MANIFEST:
-          VerifyEmptyManifest(group_->newest_complete_cache());
-          break;
-        case NONE:
-        default:
-          break;
-      }
     } else {
       EXPECT_TRUE(group_->newest_complete_cache() == NULL);
     }
@@ -901,6 +887,23 @@ class AppCacheUpdateJobTest : public testing::Test,
         }
       }
     }
+
+    // Verify expected cache contents last as some checks are asserts
+    // and will abort the test if they fail.
+    switch (tested_manifest_) {
+      case MANIFEST1:
+        VerifyManifest1(group_->newest_complete_cache());
+        break;
+      case MANIFEST_MERGED_TYPES:
+        VerifyManifestMergedTypes(group_->newest_complete_cache());
+        break;
+      case EMPTY_MANIFEST:
+        VerifyEmptyManifest(group_->newest_complete_cache());
+        break;
+      case NONE:
+      default:
+        break;
+    }
   }
 
   void VerifyManifest1(AppCache* cache) {
@@ -1145,11 +1148,11 @@ TEST_F(AppCacheUpdateJobTest, BasicCacheAttemptSuccess) {
   RunTestOnIOThread(&AppCacheUpdateJobTest::BasicCacheAttemptSuccessTest);
 }
 
-TEST_F(AppCacheUpdateJobTest, DISABLED_BasicUpgradeSuccess) {
+TEST_F(AppCacheUpdateJobTest, BasicUpgradeSuccess) {
   RunTestOnIOThread(&AppCacheUpdateJobTest::BasicUpgradeSuccessTest);
 }
 
-TEST_F(AppCacheUpdateJobTest, DISABLED_UpgradeSuccessMergedTypes) {
+TEST_F(AppCacheUpdateJobTest, UpgradeSuccessMergedTypes) {
   RunTestOnIOThread(&AppCacheUpdateJobTest::UpgradeSuccessMergedTypesTest);
 }
 
@@ -1165,7 +1168,7 @@ TEST_F(AppCacheUpdateJobTest, UpgradeFailMasterUrlFetch) {
   RunTestOnIOThread(&AppCacheUpdateJobTest::UpgradeFailMasterUrlFetchTest);
 }
 
-TEST_F(AppCacheUpdateJobTest, DISABLED_EmptyManifest) {
+TEST_F(AppCacheUpdateJobTest, EmptyManifest) {
   RunTestOnIOThread(&AppCacheUpdateJobTest::EmptyManifestTest);
 }
 
diff --git a/webkit/appcache/data/appcache_unittest/manifest-fb-404 b/webkit/appcache/data/appcache_unittest/manifest-fb-404
index aea2364..47a49a9 100644
--- a/webkit/appcache/data/appcache_unittest/manifest-fb-404
+++ b/webkit/appcache/data/appcache_unittest/manifest-fb-404
@@ -1,7 +1,7 @@
 CACHE MANIFEST
 explicit1
 FALLBACK:
-fallback404 fallback-404
 fallback1 fallback1a
+fallback404 fallback-404
 NETWORK:
 online1