Update installed ServiceWorkers.

content/browser/service_worker/service_worker_storage.cc

Index: content/browser/service_worker/service_worker_storage.cc
diff --git a/content/browser/service_worker/service_worker_storage.cc b/content/browser/service_worker/service_worker_storage.cc
index 13e0ca311b141d7124ef2c95f137d5331b6e0ea0..f09d614e55ee5c90e5e6e93af8b867348c172514 100644
--- a/content/browser/service_worker/service_worker_storage.cc
+++ b/content/browser/service_worker/service_worker_storage.cc
@@ -20,6 +20,7 @@
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/public/browser/browser_thread.h"
+#include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "webkit/browser/quota/quota_manager_proxy.h"
 
@@ -73,6 +74,130 @@ ServiceWorkerStatusCode DatabaseStatusToStatusCode(
   }
 }
 
+class CompareHelper : public base::RefCounted<CompareHelper> {

[falken, 2014/07/17 06:33:42]

would ResponseComparer be a more descriptive name? Maybe a comment like
"Compares two responses and returns the result to a CompareCallback" is good
too.

[michaeln, 2014/07/18 03:40:49]

Done (classname but not comment since the name indicates its function)

+ public:
+  CompareHelper(
+      base::WeakPtr<ServiceWorkerStorage> owner,
+      scoped_ptr<ServiceWorkerResponseReader> lhs,
+      scoped_ptr<ServiceWorkerResponseReader> rhs,
+      const ServiceWorkerStorage::CompareCallback& callback)
+      : owner_(owner),
+        completion_callback_(callback),
+        lhs_reader_(lhs.release()),
+        rhs_reader_(rhs.release()),
+        completion_count_(0),
+        previous_result_(0) {
+  }
+
+  void Start();
+
+ private:
+  friend class base::RefCounted<CompareHelper>;
+
+  static const int kBufferSize = 16 * 1024;
+
+  ~CompareHelper() {}
+  void ReadInfos();
+  void OnReadInfoComplete(int result);
+  void ReadSomeData();
+  void OnReadDataComplete(int result);
+
+  base::WeakPtr<ServiceWorkerStorage> owner_;
+  ServiceWorkerStorage::CompareCallback completion_callback_;
+  scoped_ptr<ServiceWorkerResponseReader> lhs_reader_;
+  scoped_refptr<HttpResponseInfoIOBuffer> lhs_info_;
+  scoped_refptr<net::IOBuffer> lhs_buffer_;
+  scoped_ptr<ServiceWorkerResponseReader> rhs_reader_;
+  scoped_refptr<HttpResponseInfoIOBuffer> rhs_info_;
+  scoped_refptr<net::IOBuffer> rhs_buffer_;
+  int completion_count_;
+  int previous_result_;
+  DISALLOW_COPY_AND_ASSIGN(CompareHelper);
+};
+
+void CompareHelper::Start() {
+  lhs_buffer_ = new net::IOBuffer(kBufferSize);
+  lhs_info_ = new HttpResponseInfoIOBuffer();
+  rhs_buffer_ = new net::IOBuffer(kBufferSize);
+  rhs_info_ = new HttpResponseInfoIOBuffer();
+
+  ReadInfos();
+}
+
+void CompareHelper::ReadInfos() {
+  lhs_reader_->ReadInfo(
+      lhs_info_,
+      base::Bind(&CompareHelper::OnReadInfoComplete,
+                 this));
+  rhs_reader_->ReadInfo(
+      rhs_info_,
+      base::Bind(&CompareHelper::OnReadInfoComplete,
+                 this));
+}
+
+void CompareHelper::OnReadInfoComplete(int result) {
+  if (completion_callback_.is_null() || !owner_)
+    return;
+  if (result < 0) {
+    completion_callback_.Run(SERVICE_WORKER_ERROR_FAILED, -1);
+    completion_callback_.Reset();
+    return;
+  }
+  if (++(completion_count_) != 2)

[falken, 2014/07/17 06:33:42]

nit: those parens look unnecessary to me

[michaeln, 2014/07/18 03:40:49]

Done.

+    return;
+
+  if (lhs_info_->response_data_size != rhs_info_->response_data_size) {
+    completion_callback_.Run(SERVICE_WORKER_OK, -1);

[falken, 2014/07/17 06:33:42]

nit: this seems another reason that "bool equals" instead of "int
compare_result" would be better. as written, the compare value is like memcmp
except if the sizes are different, seems arbitrary.

[michaeln, 2014/07/18 03:40:49]

Done.

+    return;
+  }
+  ReadSomeData();
+}
+
+void CompareHelper::ReadSomeData() {
+  completion_count_ = 0;
+  lhs_reader_->ReadData(
+      lhs_buffer_,
+      kBufferSize,
+      base::Bind(&CompareHelper::OnReadDataComplete, this));
+  rhs_reader_->ReadData(
+      rhs_buffer_,
+      kBufferSize,
+      base::Bind(&CompareHelper::OnReadDataComplete, this));
+}
+
+void CompareHelper::OnReadDataComplete(int result) {
+  if (completion_callback_.is_null() || !owner_)
+    return;
+  if (result < 0) {
+    completion_callback_.Run(SERVICE_WORKER_ERROR_FAILED, -1);
+    completion_callback_.Reset();
+    return;
+  }
+  if (++(completion_count_) != 2) {

[falken, 2014/07/17 06:33:42]

nit: i prefer removing those parens

[michaeln, 2014/07/18 03:40:48]

Done.

+    previous_result_ = result;
+    return;
+  }
+
+  if (result != previous_result_) {

[falken, 2014/07/17 06:33:42]

I was confused at first. I see that lhs and rhs must be the same size or we'd
abort after reading Info. But it's not possible for a single call to ReadData to
sometimes read less data than it can? I.e., if the remaining data is 10*N bytes,
and I do ReadData(N), it'll always read N?

[michaeln, 2014/07/18 03:40:49]

Good questin, I'm not sure if it's guaranteed to always return the the amount
you asked for or less as you hit eof? This doesn't look like a safe assumption,
i've added a todo.

(also i have another todo about doing this comparison as data is being retrieved
via the network instead of writing to disk and the comparing which would make
this method uneeded altogether)

+    completion_callback_.Run(SERVICE_WORKER_ERROR_FAILED, -1);
+    return;
+  }
+
+  if (result == 0) {
+    completion_callback_.Run(SERVICE_WORKER_OK, 0);
+    return;
+  }
+
+  int compare_result =
+      memcmp(lhs_buffer_->data(), rhs_buffer_->data(), result);
+  if (compare_result != 0) {
+    completion_callback_.Run(SERVICE_WORKER_OK, compare_result);
+    return;
+  }
+
+  ReadSomeData();
+}
+
 }  // namespace
 
 ServiceWorkerStorage::InitialData::InitialData()
@@ -392,6 +517,18 @@ void ServiceWorkerStorage::DoomUncommittedResponse(int64 id) {
   StartPurgingResources(std::vector<int64>(1, id));
 }
 
+void ServiceWorkerStorage::CompareScriptResources(
+    int64 lhs_id, int64 rhs_id,
+    const CompareCallback& callback) {
+  DCHECK(!callback.is_null());
+  scoped_refptr<CompareHelper> helper =
+      new CompareHelper(weak_factory_.GetWeakPtr(),
+                        CreateResponseReader(lhs_id),
+                        CreateResponseReader(rhs_id),
+                        callback);
+  helper->Start();  // It deletes itself when done.
+}
+
 void ServiceWorkerStorage::DeleteAndStartOver(const StatusCallback& callback) {
   Disable();
 
@@ -761,16 +898,19 @@ ServiceWorkerStorage::GetOrCreateRegistration(
     version->SetStatus(data.is_active ?
         ServiceWorkerVersion::ACTIVATED : ServiceWorkerVersion::INSTALLED);
     version->script_cache_map()->SetResources(resources);
+
+    // TODO(michaeln): need to activate a waiting version  that wasn't
+    // actrivated in an earlier session, maybe test for this condition
+    // (waitingversion and no activeversion) when navigating to a page?
   }
 
-  if (version->status() == ServiceWorkerVersion::ACTIVATED)
+  if (version->status() == ServiceWorkerVersion::ACTIVATED) {
     registration->SetActiveVersion(version);
-  else if (version->status() == ServiceWorkerVersion::INSTALLED)
+  } else if (version->status() == ServiceWorkerVersion::INSTALLED) {
     registration->SetWaitingVersion(version);
-  else
+  } else {
     NOTREACHED();
-  // TODO(michaeln): Hmmm, what if DeleteReg was invoked after
-  // the Find result we're returning here? NOTREACHED condition?
+  }

[falken, 2014/07/17 06:33:42]

nit: these braces aren't neeeded

[michaeln, 2014/07/18 03:40:49]

Done.

   return registration;
 }