Expose details for appcache error events [Chromium]

webkit/browser/appcache/appcache_update_job.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webkit/browser/appcache/appcache_update_job.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 58 matching lines @@
   void SendProgressNotifications(
       const GURL& url, int num_total, int num_complete) {
     for (NotifyHostMap::iterator it = hosts_to_notify.begin();
          it != hosts_to_notify.end(); ++it) {
       AppCacheFrontend* frontend = it->first;
       frontend->OnProgressEventRaised(it->second, url,
                                       num_total, num_complete);
     }
   }
 
-  void SendErrorNotifications(const std::string& error_message) {
+  void SendErrorNotifications(const std::string& error_message,
+                              const std::string& reason,
+                              const GURL& url,
+                              int status) {
     DCHECK(!error_message.empty());
     for (NotifyHostMap::iterator it = hosts_to_notify.begin();
          it != hosts_to_notify.end(); ++it) {
       AppCacheFrontend* frontend = it->first;
-      frontend->OnErrorEventRaised(it->second, error_message);
+      frontend->OnErrorEventRaised(
+          it->second, error_message, reason, url, status);
     }
   }
 
  private:
   NotifyHostMap hosts_to_notify;
 };
 
 AppCacheUpdateJob::UrlToFetch::UrlToFetch(const GURL& url,
                                           bool checked,
                                           AppCacheResponseInfo* info)
@@ skipping 318 matching lines @@
 }
 
 AppCacheResponseWriter* AppCacheUpdateJob::CreateResponseWriter() {
   AppCacheResponseWriter* writer =
       storage_->CreateResponseWriter(manifest_url_,
                                                 group_->group_id());
   stored_response_ids_.push_back(writer->response_id());
   return writer;
 }
 
-void AppCacheUpdateJob::HandleCacheFailure(
-    const std::string& error_message,
-    ResultType result) {
+void AppCacheUpdateJob::HandleCacheFailure(const std::string& error_message,
+                                           const std::string& reason,
+                                           const GURL& resource_url,
+                                           int resource_status,
+                                           ResultType result) {
   // 6.9.4 cache failure steps 2-8.
   DCHECK(internal_state_ != CACHE_FAILURE);
   DCHECK(!error_message.empty());
   DCHECK(result != UPDATE_OK);
   internal_state_ = CACHE_FAILURE;
   CancelAllUrlFetches();
-  CancelAllMasterEntryFetches(error_message);
-  NotifyAllError(error_message);
+  // TODO: Should both of these errors be generated?

[jsbell, 2014/02/21 23:48:23]

The code here will result in two errors - CancelAllMasterEntryFetches() and
NotifyAllError() both call SendErrorNotifications().

As you say, I may just be clueless about where the errors get routed to.

[michaeln, 2014/02/22 00:19:39]

Like everything... not as simple as it should be. There are multiple recipients
of the set of progress/error/yada events generated by an update job. Not all
recipients are equal, some see a different event stream than others (you have to
read the spec real close, but there it is).

What CancelAllMasterEntryFetches does is raise the error event into all pages
that caused a new master entry to be added, and then unplugs those recipients
from receiving any other events. The way in which "recipients are not equal" has
to do with whether a new master entry is being established vs a preexisting
cached page was loaded.

NotifyAllError then spams the remaining recipients with the same error.

In this particular HandleCacheFailure case, see same error is being given to all
recipeints, but there are other cases where the page with the new master will
see an error, but other pages may continue and ultimately see success. (see
HandleMasterEntryFetchCompleted)

Each recipient should see only one terminating event.

Also, a potential source of "update hangs" is bugs in this notifier plumbing
that just drops the ball and leaves some should be recipients out.

+  CancelAllMasterEntryFetches(
+      error_message, reason, resource_url, resource_status);
+  NotifyAllError(error_message, reason, resource_url, resource_status);
   DiscardInprogressCache();
   internal_state_ = COMPLETED;
   AppCacheHistograms::CountUpdateJobResult(
       result, manifest_url_.GetOrigin());
   DeleteSoon();  // To unwind the stack prior to deletion.
 }
 
 void AppCacheUpdateJob::FetchManifest(bool is_first_fetch) {
   DCHECK(!manifest_fetcher_);
   manifest_fetcher_ = new URLFetcher(
@@ skipping 47 matching lines @@
       ContinueHandleManifestFetchCompleted(true);
   } else if (response_code == 304 && update_type_ == UPGRADE_ATTEMPT) {
     ContinueHandleManifestFetchCompleted(false);
   } else if ((response_code == 404 || response_code == 410) &&
              update_type_ == UPGRADE_ATTEMPT) {
     storage_->MakeGroupObsolete(group_, this);  // async
   } else {
     const char* kFormatString = "Manifest fetch failed (%d) %s";
     std::string message = FormatUrlErrorMessage(
         kFormatString, manifest_url_, fetcher->result(), response_code);
-    HandleCacheFailure(message, fetcher->result());
+    HandleCacheFailure(message,
+                       "ManifestFetchFailed",
+                       manifest_url_,
+                       response_code,
+                       fetcher->result());
   }
 }
 
 void AppCacheUpdateJob::OnGroupMadeObsolete(AppCacheGroup* group,
                                             bool success) {
   DCHECK(master_entry_fetches_.empty());
-  CancelAllMasterEntryFetches("The cache has been made obsolete, "
-                              "the manifest file returned 404 or 410");
+  CancelAllMasterEntryFetches(
+      "The cache has been made obsolete, "
+      "the manifest file returned 404 or 410",
+      "CacheObsolete",
+      // TODO: Can we populate these correctly?
+      GURL(),
+      0);
   if (success) {
     DCHECK(group->is_obsolete());
     NotifyAllAssociatedHosts(OBSOLETE_EVENT);
     internal_state_ = COMPLETED;
     MaybeCompleteUpdate();
   } else {
     // Treat failure to mark group obsolete as a cache failure.
-    HandleCacheFailure("Failed to mark the cache as obsolete", DB_ERROR);
+    HandleCacheFailure("Failed to mark the cache as obsolete",
+                       "FailedToMarkObsolete",
+                       GURL(),
+                       0,
+                       DB_ERROR);
   }
 }
 
 void AppCacheUpdateJob::ContinueHandleManifestFetchCompleted(bool changed) {
   DCHECK(internal_state_ == FETCH_MANIFEST);
 
   if (!changed) {
     DCHECK(update_type_ == UPGRADE_ATTEMPT);
     internal_state_ = NO_UPDATE;
 
     // Wait for pending master entries to download.
     FetchMasterEntries();
     MaybeCompleteUpdate();  // if not done, run async 6.9.4 step 7 substeps
     return;
   }
 
   Manifest manifest;
   if (!ParseManifest(manifest_url_, manifest_data_.data(),
                      manifest_data_.length(), manifest)) {
     const char* kFormatString = "Failed to parse manifest %s";
     const std::string message = base::StringPrintf(kFormatString,
         manifest_url_.spec().c_str());
-    HandleCacheFailure(message, MANIFEST_ERROR);
+    HandleCacheFailure(
+        message, "InvalidManifest", manifest_url_, 0, MANIFEST_ERROR);
     VLOG(1) << message;
     return;
   }
 
   // Proceed with update process. Section 6.9.4 steps 8-20.
   internal_state_ = DOWNLOADING;
   inprogress_cache_ = new AppCache(storage_, storage_->NewCacheId());
   BuildUrlFileList(manifest);
   inprogress_cache_->InitializeWithManifest(&manifest);
 
@@ skipping 53 matching lines @@
     if (entry.IsExplicit() || entry.IsFallback() || entry.IsIntercept()) {
       if (response_code == 304 && fetcher->existing_entry().has_response_id()) {
         // Keep the existing response.
         entry.set_response_id(fetcher->existing_entry().response_id());
         entry.set_response_size(fetcher->existing_entry().response_size());
         inprogress_cache_->AddOrModifyEntry(url, entry);
       } else {
         const char* kFormatString = "Resource fetch failed (%d) %s";
         std::string message = FormatUrlErrorMessage(
             kFormatString, url, fetcher->result(), response_code);
-        HandleCacheFailure(message, fetcher->result());
+        HandleCacheFailure(message,
+                           "ResourceFetchFailed",
+                           url,
+                           response_code,
+                           fetcher->result());
         return;
       }
     } else if (response_code == 404 || response_code == 410) {
       // Entry is skipped.  They are dropped from the cache.
     } else if (update_type_ == UPGRADE_ATTEMPT &&
                fetcher->existing_entry().has_response_id()) {
       // Keep the existing response.
       // TODO(michaeln): Not sure this is a good idea. This is spec compliant
       // but the old resource may or may not be compatible with the new contents
       // of the cache. Impossible to know one way or the other.
@@ skipping 64 matching lines @@
       if (inprogress_cache_.get())
         host->AssociateNoCache(GURL());
 
       host->RemoveObserver(this);
     }
     hosts.clear();
 
     const char* kFormatString = "Manifest fetch failed (%d) %s";
     std::string message = FormatUrlErrorMessage(
         kFormatString, request->url(), fetcher->result(), response_code);
-    host_notifier.SendErrorNotifications(message);
+    host_notifier.SendErrorNotifications(
+        message, "ManifestFetchFailed", request->url(), response_code);
 
     // In downloading case, update result is different if all master entries
     // failed vs. only some failing.
     if (inprogress_cache_.get()) {
       // Only count successful downloads to know if all master entries failed.
       pending_master_entries_.erase(found);
       --master_entries_completed_;
 
       // Section 6.9.4, step 22.3.
       if (update_type_ == CACHE_ATTEMPT && pending_master_entries_.empty()) {
-        HandleCacheFailure(message, fetcher->result());
+        // TODO: Is this a duplicate error?

[michaeln, 2014/02/22 01:18:50]

No duplicates, the recipients of the error sent by
host_notifier.SendErrorNotifications on line 725 have been unplugged so to speak
(see line 716), so the error sent via HandleCacheFailure will not be delivered
to them.

+        HandleCacheFailure(message,
+                           "ManifestFetchFailed",
+                           request->url(),
+                           response_code,
+                           fetcher->result());
         return;
       }
     }
   }
 
   DCHECK(internal_state_ != CACHE_FAILURE);
   FetchMasterEntries();
   MaybeCompleteUpdate();
 }
 
@@ skipping 21 matching lines @@
           io_buffer.get(),
           base::Bind(&AppCacheUpdateJob::OnManifestInfoWriteComplete,
                      base::Unretained(this)));
     }
   } else {
     VLOG(1) << "Request status: " << request->status().status()
             << " error: " << request->status().error()
             << " response code: " << response_code;
     ScheduleUpdateRetry(kRerunDelayMs);
     if (response_code == 200) {
-      HandleCacheFailure("Manifest changed during update", MANIFEST_ERROR);
+      HandleCacheFailure("Manifest changed during update",
+                         "ManifestChanged",
+                         manifest_url_,
+                         response_code,
+                         MANIFEST_ERROR);
     } else {
       const char* kFormatString = "Manifest re-fetch failed (%d) %s";
       std::string message = FormatUrlErrorMessage(
           kFormatString, manifest_url_, fetcher->result(), response_code);
-      HandleCacheFailure(message, fetcher->result());
+      HandleCacheFailure(message,
+                         "ManifestFetchFailed",
+                         manifest_url_,
+                         response_code,
+                         fetcher->result());
     }
   }
 }
 
 void AppCacheUpdateJob::OnManifestInfoWriteComplete(int result) {
   if (result > 0) {
     scoped_refptr<net::StringIOBuffer> io_buffer(
         new net::StringIOBuffer(manifest_data_));
     manifest_response_writer_->WriteData(
         io_buffer.get(),
         manifest_data_.length(),
         base::Bind(&AppCacheUpdateJob::OnManifestDataWriteComplete,
                    base::Unretained(this)));
   } else {
     HandleCacheFailure("Failed to write the manifest headers to storage",
+                       "InternalStorageError",
+                       GURL(),
+                       0,
                        DISKCACHE_ERROR);
   }
 }
 
 void AppCacheUpdateJob::OnManifestDataWriteComplete(int result) {
   if (result > 0) {
     AppCacheEntry entry(AppCacheEntry::MANIFEST,
         manifest_response_writer_->response_id(),
         manifest_response_writer_->amount_written());
     if (!inprogress_cache_->AddOrModifyEntry(manifest_url_, entry))
       duplicate_response_ids_.push_back(entry.response_id());
     StoreGroupAndCache();
   } else {
     HandleCacheFailure("Failed to write the manifest data to storage",
+                       "InternalStorageError",
+                       GURL(),
+                       0,
                        DISKCACHE_ERROR);
   }
 }
 
 void AppCacheUpdateJob::StoreGroupAndCache() {
   DCHECK(stored_state_ == UNSTORED);
   stored_state_ = STORING;
   scoped_refptr<AppCache> newest_cache;
   if (inprogress_cache_.get())
     newest_cache.swap(inprogress_cache_);
@@ skipping 11 matching lines @@
                                                     bool success,
                                                     bool would_exceed_quota) {
   DCHECK(stored_state_ == STORING);
   if (success) {
     stored_state_ = STORED;
     MaybeCompleteUpdate();  // will definitely complete
   } else {
     // Restore inprogress_cache_ to get the proper events delivered
     // and the proper cleanup to occur.
     if (newest_cache != group->newest_complete_cache())
       inprogress_cache_ = newest_cache;

[michaeln, 2014/02/22 01:18:50]

Hmmm... unrelated to this CL, but you know the DiscardInProgressCache crasher...
i think i see how that happens now.

StoreGroupAndCache() puts the object in the state the Discard method is not
prepared for. The shutdown logic involves killing pending tasks without running
their completion functions. This completion function restores the object to the
state that DiscardInProgressCache is coded for, but it's skipped... and
DiscardInProgressCache is confused.

 
     ResultType result = DB_ERROR;
     std::string message("Failed to commit new cache to storage");
     if (would_exceed_quota) {
       message.append(", would exceed quota");
       result = QUOTA_ERROR;
     }
-    HandleCacheFailure(message, result);
+    HandleCacheFailure(
+        message,
+        would_exceed_quota ? "QuotaError" : "InternalStorageError",
+        GURL(),
+        0,
+        result);
   }
 }
 
 void AppCacheUpdateJob::NotifySingleHost(AppCacheHost* host,
                                          EventID event_id) {
   std::vector<int> ids(1, host->host_id());
   host->frontend()->OnEventRaised(ids, event_id);
 }
 
 void AppCacheUpdateJob::NotifyAllAssociatedHosts(EventID event_id) {
   HostNotifier host_notifier;
   AddAllAssociatedHostsToNotifier(&host_notifier);
   host_notifier.SendNotifications(event_id);
 }
 
 void AppCacheUpdateJob::NotifyAllProgress(const GURL& url) {
   HostNotifier host_notifier;
   AddAllAssociatedHostsToNotifier(&host_notifier);
   host_notifier.SendProgressNotifications(
       url, url_file_list_.size(), url_fetches_completed_);
 }
 
 void AppCacheUpdateJob::NotifyAllFinalProgress() {
   DCHECK(url_file_list_.size() == url_fetches_completed_);
   NotifyAllProgress(GURL());
 }
 
-void AppCacheUpdateJob::NotifyAllError(const std::string& error_message) {
+void AppCacheUpdateJob::NotifyAllError(const std::string& error_message,
+                                       const std::string& reason,
+                                       const GURL& resource_url,
+                                       int resource_status) {
   HostNotifier host_notifier;
   AddAllAssociatedHostsToNotifier(&host_notifier);
-  host_notifier.SendErrorNotifications(error_message);
+  host_notifier.SendErrorNotifications(
+      error_message, reason, resource_url, resource_status);
 }
 
 void AppCacheUpdateJob::AddAllAssociatedHostsToNotifier(
     HostNotifier* host_notifier) {
   // Collect hosts so we only send one notification per frontend.
   // A host can only be associated with a single cache so no need to worry
   // about duplicate hosts being added to the notifier.
   if (inprogress_cache_.get()) {
     DCHECK(internal_state_ == DOWNLOADING || internal_state_ == CACHE_FAILURE);
     host_notifier->AddHosts(inprogress_cache_->associated_hosts());
@@ skipping 34 matching lines @@
   AppCacheEntry* entry = NULL;
   if (group_->newest_complete_cache())
     entry = group_->newest_complete_cache()->GetEntry(manifest_url_);
   if (!entry) {
     // TODO(michaeln): This is just a bandaid to avoid a crash.
     // http://code.google.com/p/chromium/issues/detail?id=95101
     if (service_->storage() == storage_) {
       // Use a local variable because service_ is reset in HandleCacheFailure.
       AppCacheService* service = service_;
       HandleCacheFailure("Manifest entry not found in existing cache",
+                         "InternalStorageError",
+                         GURL(),
+                         0,
                          DB_ERROR);
       AppCacheHistograms::AddMissingManifestEntrySample();
       service->DeleteAppCacheGroup(manifest_url_, net::CompletionCallback());
     }
     return;
   }
 
   // Load manifest data from storage to compare against fetched manifest.
   manifest_response_reader_.reset(
       storage_->CreateResponseReader(manifest_url_,
@@ skipping 221 matching lines @@
           url, URLFetcher::MASTER_ENTRY_FETCH, this);
       fetcher->Start();
       master_entry_fetches_.insert(PendingUrlFetches::value_type(url, fetcher));
     }
 
     master_entries_to_fetch_.erase(master_entries_to_fetch_.begin());
   }
 }
 
 void AppCacheUpdateJob::CancelAllMasterEntryFetches(
-    const std::string& error_message) {
+    const std::string& error_message,
+    const std::string& reason,
+    const GURL& resource_url,
+    int resource_status) {
   // For now, cancel all in-progress fetches for master entries and pretend
   // all master entries fetches have completed.
   // TODO(jennb): Delete this when update no longer fetches master entries
   // directly.
 
   // Cancel all in-progress fetches.
   for (PendingUrlFetches::iterator it = master_entry_fetches_.begin();
        it != master_entry_fetches_.end(); ++it) {
     delete it->second;
     master_entries_to_fetch_.insert(it->first);  // back in unfetched list
@@ skipping 15 matching lines @@
          host_it != hosts.end(); ++host_it) {
       AppCacheHost* host = *host_it;
       host->AssociateNoCache(GURL());
       host_notifier.AddHost(host);
       host->RemoveObserver(this);
     }
     hosts.clear();
 
     master_entries_to_fetch_.erase(master_entries_to_fetch_.begin());
   }
-  host_notifier.SendErrorNotifications(error_message);
+  host_notifier.SendErrorNotifications(
+      error_message, reason, resource_url, resource_status);
 }
 
 bool AppCacheUpdateJob::MaybeLoadFromNewestCache(const GURL& url,
                                                  AppCacheEntry& entry) {
   if (update_type_ != UPGRADE_ATTEMPT)
     return false;
 
   AppCache* newest = group_->newest_complete_cache();
   AppCacheEntry* copy_me = newest->GetEntry(url);
   if (!copy_me || !copy_me->has_response_id())
@@ skipping 211 matching lines @@
 
   // Break the connection with the group so the group cannot call delete
   // on this object after we've posted a task to delete ourselves.
   group_->SetUpdateStatus(AppCacheGroup::IDLE);
   group_ = NULL;
 
   base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
 
 }  // namespace appcache