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 29 matching lines @@
                          net::LOAD_DISABLE_INTERCEPT);
   if (existing_response_headers_.get())
     AddConditionalHeaders(existing_response_headers_.get());
   request_->Start();
 }
 
 void AppCacheUpdateJob::URLFetcher::OnReceivedRedirect(
     net::URLRequest* request, const GURL& new_url,  bool* defer_redirect) {
   DCHECK(request_ == request);
   // Redirect is not allowed by the update process.
   request->Cancel();

[michaeln, 2014/03/01 00:20:02]

It might be a matter of not calling cancel here, so the status code shows thru
down under OnResponseCompleted in the Job method. OnResponseCompleted deletes
the request which will cancel things as well.

   result_ = REDIRECT_ERROR;
   OnResponseCompleted();
 }
 
 void AppCacheUpdateJob::URLFetcher::OnResponseStarted(
     net::URLRequest *request) {
   DCHECK(request == request_);
   int response_code = -1;
   if (request->status().is_success())
     response_code = request->GetResponseCode();
@@ skipping 268 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);
+  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_OBSOLETE,
+      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",
+                       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);

[michaeln, 2014/03/01 00:20:02]

maybe use an empty GURL() here too?

[jsbell, 2014/03/01 00:53:07]

Will do.

     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());
+        ResultType result = fetcher->result();
+        HandleCacheFailure(
+            message,
+            result == DISKCACHE_ERROR ? ERROR_INTERNAL : ERROR_RESOURCE_FETCH,

[michaeln, 2014/03/01 00:20:02]

This looks reasonable to me, there is something that needs to be checked. In the
REDIRECT_ERROR case, i'm not sure what value will be reported for response_code?
We'll need to be sure that the 30x shows up in that case. Also i'm not sure what
value will show up in response_code for for DISKCACHE_ERROR, less important what
gets put in response_code in that case i suppose?

That's what the ?'s meant here...

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 = ?)

[jsbell, 2014/03/01 00:53:07]

D'oh, right. I'll fix.

BTW, I'm making the minimal/obvious changes, but I'm not dismissing the overall
discussion.

+            url,
+            response_code,
+            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 16 matching lines @@
   // TODO(jennb): Handle downloads completing during cache failure when update
   // no longer fetches master entries directly. For now, we cancel all pending
   // master entry fetches when entering cache failure state so this will never
   // be called in CACHE_FAILURE state.
 
   net::URLRequest* request = fetcher->request();
   const GURL& url = request->original_url();
   master_entry_fetches_.erase(url);
   ++master_entries_completed_;
 
   int response_code = request->status().is_success()

[michaeln, 2014/03/01 00:20:02]

in cases when the fetcher cancels the request, i think we'll be reporting -1 for
status code. upon seeing a redirect, i think the fetcher cancels

[jsbell, 2014/03/01 00:53:07]

Okay, I'll try and trace this through. We'll need tests, obviously.

[michaeln, 2014/03/01 01:34:56]

There are a lot of updatejob unittests. Checking of expected values could
probably be added to those tests.

Whoa... I think I might have an unexpected explanation for some of the 'hung'
updates. Look at CacheAttemptFetchManifestFailTest and look at the set of
expected events... only CHECKING. This tests has that expectation, got me
wondering if this (screwed up) behavior might actually be the specified
behavior?

[jsbell, 2014/03/03 23:10:18]

Nice find. Hrm.... spec's table (6.7.1.2 Event summary) says that 'checking'
will be followed by one of 'noupdate', 'downloading', 'obsolete' or 'error', but
the spec may be inconsistent. ManifestRedirectTest also looks like it's only
expecting 'checking' ?

Though it looks like the event is cancelable, although I don't see the test
triggering that in any way.

Spec certainly seems to indicate that 404/410 should fire 'obsolete',  other 4xx
or 5xx or parse failure 'error', 304 (or byte-wise identical) as 'noupdate',
otherwise 'downloading'

       ? request->GetResponseCode() : -1;
 
   PendingMasters::iterator found = pending_master_entries_.find(url);
   DCHECK(found != pending_master_entries_.end());
   PendingHosts& hosts = found->second;
 
   // Section 6.9.4. No update case: step 7.3, else step 22.
   if (response_code / 100 == 2) {
     // Add fetched master entry to the appropriate cache.
     AppCache* cache = inprogress_cache_.get() ? inprogress_cache_.get()
@@ skipping 27 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());
+        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