ServiceWorker: More accurate StartWorker result and UMA

content/browser/service_worker/service_worker_version.h

 // Copyright 2013 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.
 
 #ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_
 #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_
 
 #include <map>
 #include <queue>
 #include <set>
@@ skipping 268 matching lines @@
   void AddListener(Listener* listener);
   void RemoveListener(Listener* listener);
 
   ServiceWorkerScriptCacheMap* script_cache_map() { return &script_cache_map_; }
   EmbeddedWorkerInstance* embedded_worker() { return embedded_worker_.get(); }
 
   // Reports the error message to |listeners_|.
   void ReportError(ServiceWorkerStatusCode status,
                    const std::string& status_message);
 
+  // Sets the status code to pass to StartWorker callbacks if start fails.
+  void SetStartWorkerStatusCode(ServiceWorkerStatusCode status);
+
   // Sets this version's status to REDUNDANT and deletes its resources.
   // The version must not have controllees.
   void Doom();
   bool is_redundant() const { return status_ == REDUNDANT; }
 
   bool skip_waiting() const { return skip_waiting_; }
 
   bool force_bypass_cache_for_scripts() {
     return force_bypass_cache_for_scripts_;
   }
@@ skipping 237 matching lines @@
   // callback map).
   std::queue<RequestInfo> requests_;
 
   bool skip_waiting_ = false;
   bool skip_recording_startup_time_ = false;
   bool force_bypass_cache_for_scripts_ = false;
 
   std::vector<int> pending_skip_waiting_requests_;
   scoped_ptr<net::HttpResponseInfo> main_script_http_info_;
 
+  // The status when StartWorker was invoked. Used for UMA.
+  Status prestart_status_ = NEW;

[nhiroki, 2015/04/24 04:58:28]

How about binding "prestart status" to DidEnsureLiveRegistrationForStartWorker
and RecordStartWorkerResult, and then removing this field? This class already
has many states and I'd rather not add such fields more...

[falken, 2015/04/24 05:08:26]

I originally wanted to do this but there are RecordStartWorkerResult callsites
like in the dtor that aren't binded. I agree about the states, maybe there's
opportunities to split some of these out into helper classes later.

[nhiroki, 2015/04/24 05:23:57]

Oh... I see. Thank you for explanation. LGTM

+  // If not OK, the reason that StartWorker failed. Used for
+  // running |start_callbacks_|.
+  ServiceWorkerStatusCode start_worker_status_ = SERVICE_WORKER_OK;
+
   base::WeakPtrFactory<ServiceWorkerVersion> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerVersion);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_VERSION_H_