Fixes to address the failing appcache/remove-cache.html layout tests.

BUG=31300
TEST=yes, layout test

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

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

6 files changed, 20 insertions, 15 deletions

diff --git a/webkit/appcache/appcache_group.cc b/webkit/appcache/appcache_group.cc
index a9ad808..e5511a6 100644
--- a/webkit/appcache/appcache_group.cc
+++ b/webkit/appcache/appcache_group.cc
@@ -171,7 +171,9 @@ void AppCacheGroup::RunQueuedUpdates() {
       queued_observers_.RemoveObserver(host);
       observers_.AddObserver(host);
     }
-    StartUpdateWithNewMasterEntry(host, it->second);
+
+    if (!is_obsolete())
+      StartUpdateWithNewMasterEntry(host, it->second);
   }
 }
 
@@ -189,7 +191,7 @@ bool AppCacheGroup::FindObserver(UpdateObserver* find_me,
 void AppCacheGroup::ScheduleUpdateRestart(int delay_ms) {
   DCHECK(!restart_update_task_);
   restart_update_task_ =
-    NewRunnableMethod(this, &AppCacheGroup::RunQueuedUpdates);
+      NewRunnableMethod(this, &AppCacheGroup::RunQueuedUpdates);
   MessageLoop::current()->PostDelayedTask(FROM_HERE, restart_update_task_,
                                           delay_ms);
 }
diff --git a/webkit/appcache/appcache_host.cc b/webkit/appcache/appcache_host.cc
index 489d062..d35d61a 100644
--- a/webkit/appcache/appcache_host.cc
+++ b/webkit/appcache/appcache_host.cc
@@ -269,14 +269,17 @@ void AppCacheHost::FinishCacheSelection(
     DCHECK(cache->owning_group());
     DCHECK(new_master_entry_url_.is_empty());
     AssociateCache(cache);
-    cache->owning_group()->StartUpdateWithHost(this);
-    ObserveGroupBeingUpdated(cache->owning_group());
+    if (!cache->owning_group()->is_obsolete()) {
+      cache->owning_group()->StartUpdateWithHost(this);
+      ObserveGroupBeingUpdated(cache->owning_group());
+    }
   } else if (group) {
     // If document was loaded using HTTP GET or equivalent, and, there is a
     // manifest URL, and manifest URL has the same origin as document.
     // Invoke the application cache update process for manifest URL, with
     // the browsing context being navigated, and with document and the
     // resource from which document was loaded as the new master resourse.
+    DCHECK(!group->is_obsolete());
     DCHECK(new_master_entry_url_.is_valid());
     AssociateCache(NULL);  // The UpdateJob may produce one for us later.
     group->StartUpdateWithNewMasterEntry(this, new_master_entry_url_);
diff --git a/webkit/appcache/appcache_storage_impl.cc b/webkit/appcache/appcache_storage_impl.cc
index 579a6bf..cfa3be9 100644
--- a/webkit/appcache/appcache_storage_impl.cc
+++ b/webkit/appcache/appcache_storage_impl.cc
@@ -583,6 +583,11 @@ void AppCacheStorageImpl::MakeGroupObsoleteTask::RunCompleted() {
   if (success_) {
     storage_->origins_with_groups_.swap(origins_with_groups_);
     group_->set_obsolete(true);
+
+    // Also remove from the working set, caches for an 'obsolete' group
+    // may linger in use, but the group itself cannot be looked up by
+    // 'manifest_url' in the working set any longer.
+    storage_->working_set()->RemoveGroup(group_);
   }
   FOR_EACH_DELEGATE(delegates_, OnGroupMadeObsolete(group_, success_));
   group_ = NULL;
diff --git a/webkit/appcache/appcache_update_job.cc b/webkit/appcache/appcache_update_job.cc
index dbd763f8..fbe79ff 100644
--- a/webkit/appcache/appcache_update_job.cc
+++ b/webkit/appcache/appcache_update_job.cc
@@ -133,6 +133,7 @@ AppCacheUpdateJob::~AppCacheUpdateJob() {
 void AppCacheUpdateJob::StartUpdate(AppCacheHost* host,
                                     const GURL& new_master_resource) {
   DCHECK(group_->update_job() == this);
+  DCHECK(!group_->is_obsolete());
 
   bool is_new_pending_master_entry = false;
   if (!new_master_resource.is_empty()) {
diff --git a/webkit/appcache/mock_appcache_storage.cc b/webkit/appcache/mock_appcache_storage.cc
index 44bcd12..64a66a7 100644
--- a/webkit/appcache/mock_appcache_storage.cc
+++ b/webkit/appcache/mock_appcache_storage.cc
@@ -359,6 +359,11 @@ void MockAppCacheStorage::ProcessMakeGroupObsolete(
 
   group->set_obsolete(true);
 
+  // Also remove from the working set, caches for an 'obsolete' group
+  // may linger in use, but the group itself cannot be looked up by
+  // 'manifest_url' in the working set any longer.
+  working_set()->RemoveGroup(group);
+
   if (delegate_ref->delegate)
     delegate_ref->delegate->OnGroupMadeObsolete(group, true);
 }
@@ -406,10 +411,6 @@ void MockAppCacheStorage::AddStoredGroup(AppCacheGroup* group) {
 }
 
 void MockAppCacheStorage::RemoveStoredGroup(AppCacheGroup* group) {
-  // Also remove from the working set, caches for an 'obsolete' group
-  // may linger in use, but the group itself cannot be looked up by
-  // 'manifest_url' in the working set any longer.
-  working_set()->RemoveGroup(group);
   stored_groups_.erase(group->manifest_url());
 }
 
diff --git a/webkit/tools/layout_tests/test_expectations.txt b/webkit/tools/layout_tests/test_expectations.txt
index 00bcbc5..cf0c0fe 100644
--- a/webkit/tools/layout_tests/test_expectations.txt
+++ b/webkit/tools/layout_tests/test_expectations.txt
@@ -2821,15 +2821,8 @@ BUG9798 LINUX : LayoutTests/http/tests/security/clipboard/clipboard-file-access.
 BUG2844 : LayoutTests/http/tests/appcache/cyrillic-uri.html = TIMEOUT PASS
 BUG2844 : LayoutTests/http/tests/appcache/fail-on-update.html = TIMEOUT PASS TEXT
 BUG2844 : LayoutTests/http/tests/appcache/offline-access.html = TIMEOUT PASS
-BUG2844 LINUX: LayoutTests/http/tests/appcache/remove-cache.html = TIMEOUT PASS
-BUG2844 WIN RELEASE : LayoutTests/http/tests/appcache/remove-cache.html = TIMEOUT PASS
-BUG9798 WIN DEBUG : LayoutTests/http/tests/appcache/remove-cache.html = TIMEOUT PASS TEXT
 BUG2844 : LayoutTests/http/tests/appcache/update-cache.html = TIMEOUT PASS
 
-// Need to investigate this one, started failing mac only with r35354
-// which switched to using a sql based storage impl.
-BUG31300 SKIP MAC : LayoutTests/http/tests/appcache/remove-cache.html = FAIL
-
 // These tests sometimes pass and sometimes have a text difference due
 // to chromium's appcache being async.
 BUG2844 WIN RELEASE : LayoutTests/http/tests/appcache/top-frame-3.html = TEXT PASS