more uma stats

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 113 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.
+  job_->MadeProgress();

[jsbell, 2014/03/21 16:41:02]

So here, a redirect - which is an error - calls MakeProgress

[michaeln, 2014/03/24 18:12:04]

Yes, MadeProgress() just means it wasn't dead in a hung state, having received a
response of any kind means the network was functioning and the algorithm was
moving forward (albeit not towards a successful completion).

Does that make sense?

[jsbell, 2014/03/24 22:21:23]

Yep, thanks.

   request->Cancel();
   result_ = REDIRECT_ERROR;
   OnResponseCompleted();
 }
 
 void AppCacheUpdateJob::URLFetcher::OnResponseStarted(
     net::URLRequest *request) {
   DCHECK(request == request_);
   int response_code = -1;
-  if (request->status().is_success())
+  if (request->status().is_success()) {
     response_code = request->GetResponseCode();
+    job_->MadeProgress();

[jsbell, 2014/03/21 16:41:02]

But here and below, MakeProgress is only called if the response was successful.

[michaeln, 2014/03/24 18:12:04]

see previous comment

+  }
   if ((response_code / 100) == 2) {
 
     // See http://code.google.com/p/chromium/issues/detail?id=69594
     // We willfully violate the HTML5 spec at this point in order
     // to support the appcaching of cross-origin HTTPS resources.
     // We've opted for a milder constraint and allow caching unless
     // the resource has a "no-store" header. A spec change has been
     // requested on the whatwg list.
     // TODO(michaeln): Consider doing this for cross-origin HTTP resources too.
     if (url_.SchemeIsSecure() &&
@@ skipping 27 matching lines @@
       result_ = NETWORK_ERROR;
     OnResponseCompleted();
   }
 }
 
 void AppCacheUpdateJob::URLFetcher::OnReadCompleted(
     net::URLRequest* request, int bytes_read) {
   DCHECK(request_ == request);
   bool data_consumed = true;
   if (request->status().is_success() && bytes_read > 0) {
+    job_->MadeProgress();
     data_consumed = ConsumeResponseData(bytes_read);
     if (data_consumed) {
       bytes_read = 0;
       while (request->Read(buffer_.get(), kBufferSize, &bytes_read)) {
         if (bytes_read > 0) {
           data_consumed = ConsumeResponseData(bytes_read);
           if (!data_consumed)
             break;  // wait for async data processing, then read more
         } else {
           break;
@@ skipping 70 matching lines @@
           bytes_read,
           base::Bind(&URLFetcher::OnWriteComplete, base::Unretained(this)));
       return false;  // wait for async write completion to continue reading
     default:
       NOTREACHED();
   }
   return true;
 }
 
 void AppCacheUpdateJob::URLFetcher::OnResponseCompleted() {
+  if (request_->status().is_success())
+    job_->MadeProgress();
+
   // Retry for 503s where retry-after is 0.
   if (request_->status().is_success() &&
       request_->GetResponseCode() == 503 &&
       MaybeRetryRequest()) {
     return;
   }
 
   switch (fetch_type_) {
     case MANIFEST_FETCH:
       job_->HandleManifestFetchCompleted(this);
@@ skipping 95 matching lines @@
       // Add to fetch list or an existing entry if already fetched.
       if (!new_master_resource.is_empty()) {
         AddMasterEntryToFetchList(host, new_master_resource,
                                   is_new_pending_master_entry);
       }
     }
     return;
   }
 
   // Begin update process for the group.
+  MadeProgress();
   group_->SetUpdateStatus(AppCacheGroup::CHECKING);
   if (group_->HasCache()) {
     update_type_ = UPGRADE_ATTEMPT;
     NotifyAllAssociatedHosts(CHECKING_EVENT);
   } else {
     update_type_ = CACHE_ATTEMPT;
     DCHECK(host);
     NotifySingleHost(host, CHECKING_EVENT);
   }
 
   if (!new_master_resource.is_empty()) {
     AddMasterEntryToFetchList(host, new_master_resource,
                               is_new_pending_master_entry);
   }
 
   FetchManifest(true);
 }
 
 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) {
+    ResultType result,
+    const GURL& failed_resource_url) {
   // 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;
+  LogHistogramStats(result, failed_resource_url);
   CancelAllUrlFetches();
   CancelAllMasterEntryFetches(error_message);
   NotifyAllError(error_message);
   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(
      manifest_url_,
      is_first_fetch ? URLFetcher::MANIFEST_FETCH :
                       URLFetcher::MANIFEST_REFETCH,
      this);
@@ skipping 23 matching lines @@
     URLFetcher* fetcher) {
   DCHECK_EQ(internal_state_, FETCH_MANIFEST);
   DCHECK_EQ(manifest_fetcher_, fetcher);
   manifest_fetcher_ = NULL;
 
   net::URLRequest* request = fetcher->request();
   int response_code = -1;
   bool is_valid_response_code = false;
   if (request->status().is_success()) {
     response_code = request->GetResponseCode();
-    is_valid_response_code = (response_code / 100 == 2);
+    is_valid_response_code = (response_code / 100 == 2);;

[jsbell, 2014/03/21 16:41:02]

Extra ; - was this going to MakeProgress too?

[michaeln, 2014/03/24 18:12:04]

Done. idk, maybe i had it that way before the fetcher started updating the time
value for all network activity?

   }
 
   if (is_valid_response_code) {
     manifest_data_ = fetcher->manifest_data();
     manifest_response_info_.reset(
         new net::HttpResponseInfo(request->response_info()));
     if (update_type_ == UPGRADE_ATTEMPT)
       CheckIfManifestChanged();  // continues asynchronously
     else
       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, fetcher->result(), GURL());
   }
 }
 
 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");
   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",
+                       DB_ERROR, GURL());
   }
 }
 
 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, MANIFEST_ERROR, GURL());
     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, fetcher->result(), url);
         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 75 matching lines @@
 
     // 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, fetcher->result(), GURL());
         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", MANIFEST_ERROR, GURL());
     } 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, fetcher->result(), GURL());
     }
   }
 }
 
 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",
-                       DISKCACHE_ERROR);
+                       DISKCACHE_ERROR, GURL());
   }
 }
 
 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",
-                       DISKCACHE_ERROR);
+                       DISKCACHE_ERROR, GURL());
   }
 }
 
 void AppCacheUpdateJob::StoreGroupAndCache() {
   DCHECK(stored_state_ == UNSTORED);
   stored_state_ = STORING;
   scoped_refptr<AppCache> newest_cache;
   if (inprogress_cache_.get())
     newest_cache.swap(inprogress_cache_);
   else
@@ skipping 20 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, result, GURL());
   }
 }
 
 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) {
@@ skipping 65 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",
-                         DB_ERROR);
+                         DB_ERROR, GURL());
       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_,
                                      group_->group_id(),
@@ skipping 383 matching lines @@
       break;
     case REFETCH_MANIFEST:
       DCHECK(stored_state_ == STORED);
       NotifyAllFinalProgress();
       if (update_type_ == CACHE_ATTEMPT)
         NotifyAllAssociatedHosts(CACHED_EVENT);
       else
         NotifyAllAssociatedHosts(UPDATE_READY_EVENT);
       DiscardDuplicateResponses();
       internal_state_ = COMPLETED;
-      AppCacheHistograms::CountUpdateJobResult(
-          UPDATE_OK, manifest_url_.GetOrigin());
+      LogHistogramStats(UPDATE_OK, GURL());
       break;
     case CACHE_FAILURE:
       NOTREACHED();  // See HandleCacheFailure
       break;
     default:
       break;
   }
 
   // Let the stack unwind before deletion to make it less risky as this
   // method is called from multiple places in this file.
   if (internal_state_ == COMPLETED)
     DeleteSoon();
 }
 
 void AppCacheUpdateJob::ScheduleUpdateRetry(int delay_ms) {
   // TODO(jennb): post a delayed task with the "same parameters" as this job
   // to retry the update at a later time. Need group, URLs of pending master
   // entries and their hosts.
 }
 
 void AppCacheUpdateJob::Cancel() {
   internal_state_ = CANCELLED;
 
+  LogHistogramStats(CANCELLED_ERROR, GURL());

[michaeln, 2014/03/21 01:21:39]

When the job ends by this code path, if there are associated hosts, they aren't
told it. I should make a distinction between hosts vs nohosts here in the stats
(two different completion codes i guess). The latter is expected if the user
navigates away from a page with a job in progress. The former is what happens in
the forcible delete cases.

KILLED vs ABANDONED (or something)

+
   if (manifest_fetcher_) {
     delete manifest_fetcher_;
     manifest_fetcher_ = NULL;
   }
 
   for (PendingUrlFetches::iterator it = pending_url_fetches_.begin();
        it != pending_url_fetches_.end(); ++it) {
     delete it->second;
   }
   pending_url_fetches_.clear();
@@ skipping 56 matching lines @@
     (*hosts.begin())->AssociateNoCache(GURL());
 
   inprogress_cache_ = NULL;
   added_master_entries_.clear();
 }
 
 void AppCacheUpdateJob::DiscardDuplicateResponses() {
   storage_->DoomResponses(manifest_url_, duplicate_response_ids_);
 }
 
+void AppCacheUpdateJob::LogHistogramStats(
+      ResultType result, const GURL& failed_resource_url) {
+  AppCacheHistograms::CountUpdateJobResult(result, manifest_url_.GetOrigin());
+  if (result == UPDATE_OK)
+    return;
+
+  int percent_complete = 0;
+  if (url_file_list_.size() > 0) {
+    size_t actual_fetches_completed = url_fetches_completed_;
+    if (!failed_resource_url.is_empty() && actual_fetches_completed)
+      --actual_fetches_completed;
+    percent_complete = (static_cast<double>(actual_fetches_completed) /
+                            static_cast<double>(url_file_list_.size())) * 100.0;
+    percent_complete = std::min(percent_complete, 99);
+  }
+
+  bool was_making_progress =
+      base::Time::Now() - last_progress_time_ <
+          base::TimeDelta::FromMinutes(5);
+
+  bool off_origin_resource_failure =
+      !failed_resource_url.is_empty() &&
+          (failed_resource_url.GetOrigin() != manifest_url_.GetOrigin());
+
+  AppCacheHistograms::LogUpdateFailureStats(
+      manifest_url_.GetOrigin(),
+      percent_complete,
+      was_making_progress,
+      off_origin_resource_failure);
+}
+
 void AppCacheUpdateJob::DeleteSoon() {
   ClearPendingMasterEntries();
   manifest_response_writer_.reset();
   storage_->CancelDelegateCallbacks(this);
   service_->RemoveObserver(this);
   service_ = NULL;
 
   // 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