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,
+                              appcache::ErrorReason 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,
+                                           ErrorReason 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?

[michaeln, 2014/02/27 23:16:06]

This should be ok, CancelAllMasterEntryFetches is delivered to one set of
clients and NotifyAllError is sent to another. No client sees two errors, and
the same error is seen by both sets.

+  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,
+                       ERROR_MANIFEST_FETCH,
+                       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",
+      ERROR_MANIFEST_FETCH,

[michaeln, 2014/02/27 23:16:06]

s/b ERROR_MANIFEST_OBSOLETE?

This is another of those differences in behavior
- new master entry pages see and error event
- existing cached pages see an obsolete event

[jsbell, 2014/02/28 22:46:00]

Whoops, yes.
Ah, thanks.

+      // TODO: Can we populate these correctly?
+      GURL(),

[michaeln, 2014/02/27 23:16:06]

Maybe we shouldn't populate the url attribute for all reasons? If the reason is
particular to the manifesturl thats should be a known enough value to the
caller.

manifest_url_ should be available here, but the result code is gone at this
point

see const GURL manifest_url_;  // here for easier access

[jsbell, 2014/02/28 22:46:00]

Agreed. Given that the message has "404 or 410" I was wondering if we could pipe
it through, but since we can't then we don't need to send the URL along either.

+      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",
+                       ERROR_INTERNAL,
+                       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, ERROR_MANIFEST_INVALID, 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,

[michaeln, 2014/02/27 23:16:06]

There are a handful of reasons a fetcher could have failed, fetcher->result()
can contain these kinds of errors

REDIRECT_ERROR (got a 30x which is verbotten, response_code = ?)
SERVER_ERROR (got non-200 response, response_code > 0)
NETWORK_ERROR (got no response whatsoever, response_code = -1)
DISKCACHE_ERROR (failed to write what we got to disk, response_code = ?)

What do we want to say about 'reason' to in the resulting error event?

ERROR_RESOURCE_FETCH definitely describes the operation or step that went wrong,
but thats a little different than the reason for failure. The consumer might be
able to discern the 'reason' based on the value in response_code.

I'm not sure what we want to do here, just bringing it up. If 'message' is also
exposed finer grained distinctions could be made there.

[jsbell, 2014/02/28 22:46:00]

^^^ combined with response_code, I think a single type for those is sufficient.
(i.e. if it's a 5xx they know the server is busted, 4xx they know the config is
wrong, 3xx they should read the spec.)

Then again: if redirect is explicitly called out in the spec, having a distinct
error type for it makes sense. Thoughts?
^^^ Makes sense to distinguish in some way. Or we could say ERROR_RESOURCE_FETCH
and the response_code is 0 ? i.e. this is the same basic category of error.
And this seems like it's the ERROR_INTERNAL bucket, i.e. you as a developer
can't fix this. 
That's what I was leaning towards, apart from the cache failure.

+                           ERROR_RESOURCE_FETCH,
+                           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, ERROR_MANIFEST_FETCH, 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?
+        HandleCacheFailure(message,
+                           ERROR_MANIFEST_FETCH,
+                           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",
+                         ERROR_MANIFEST_CHANGED,
+                         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,
+                         ERROR_MANIFEST_FETCH,
+                         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",
+                       ERROR_INTERNAL,
+                       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",
+                       ERROR_INTERNAL,
+                       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 21 matching lines @@
     // and the proper cleanup to occur.
     if (newest_cache != group->newest_complete_cache())
       inprogress_cache_ = newest_cache;
 
     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 ? ERROR_QUOTA_EXCEEDED : ERROR_INTERNAL,
+        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,
+                                       ErrorReason 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",
+                         ERROR_INTERNAL,
+                         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,
+    ErrorReason 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 223 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