Fix bug in identification of ServiceWorker versions in chrome://serviceworker_internals.

Because version ids are unique within a storage partition but not across them, verion_id by itself is not sufficient to identify a version.

BUG=359517

Review URL: https://codereview.chromium.org/242923002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@264782 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 94 insertions, 80 deletions

diff --git a/content/browser/service_worker/service_worker_internals_ui.cc b/content/browser/service_worker/service_worker_internals_ui.cc
index 05daa31..b982a63 100644
--- a/content/browser/service_worker/service_worker_internals_ui.cc
+++ b/content/browser/service_worker/service_worker_internals_ui.cc
@@ -8,8 +8,10 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/memory/scoped_vector.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
+#include "content/browser/service_worker/service_worker_context_observer.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_version.h"
@@ -41,7 +43,8 @@ class ServiceWorkerInternalsUI::OperationProxy
                  scoped_ptr<ListValue> original_args)
       : internals_(internals), original_args_(original_args.Pass()) {}
 
-  void GetRegistrationsOnIOThread(ServiceWorkerContextWrapper* context,
+  void GetRegistrationsOnIOThread(int partition_id,
+                                  ServiceWorkerContextWrapper* context,
                                   const base::FilePath& context_path);
   void UnregisterOnIOThread(scoped_refptr<ServiceWorkerContextWrapper> context,
                             const GURL& scope);
@@ -57,6 +60,7 @@ class ServiceWorkerInternalsUI::OperationProxy
   friend class base::RefCountedThreadSafe<OperationProxy>;
   ~OperationProxy() {}
   void OnHaveRegistrations(
+      int partition_id,
       const base::FilePath& context_path,
       const std::vector<ServiceWorkerRegistrationInfo>& registrations);
 
@@ -78,8 +82,70 @@ class ServiceWorkerInternalsUI::OperationProxy
   scoped_ptr<ListValue> original_args_;
 };
 
+class ServiceWorkerInternalsUI::PartitionObserver
+    : public ServiceWorkerContextObserver {
+ public:
+  PartitionObserver(int partition_id, WebUI* web_ui)
+      : partition_id_(partition_id), web_ui_(web_ui) {}
+  virtual ~PartitionObserver() {}
+  // ServiceWorkerContextObserver overrides:
+  virtual void OnWorkerStarted(int64 version_id,
+                               int process_id,
+                               int thread_id) OVERRIDE {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    web_ui_->CallJavascriptFunction(
+        "serviceworker.onWorkerStarted",
+        FundamentalValue(partition_id_),
+        StringValue(base::Int64ToString(version_id)),
+        FundamentalValue(process_id),
+        FundamentalValue(thread_id));
+  }
+  virtual void OnWorkerStopped(int64 version_id,
+                               int process_id,
+                               int thread_id) OVERRIDE {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    web_ui_->CallJavascriptFunction(
+        "serviceworker.onWorkerStopped",
+        FundamentalValue(partition_id_),
+        StringValue(base::Int64ToString(version_id)),
+        FundamentalValue(process_id),
+        FundamentalValue(thread_id));
+  }
+  virtual void OnVersionStateChanged(int64 version_id) OVERRIDE {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    web_ui_->CallJavascriptFunction(
+        "serviceworker.onVersionStateChanged",
+        FundamentalValue(partition_id_),
+        StringValue(base::Int64ToString(version_id)));
+  }
+  virtual void OnErrorReported(int64 version_id,
+                               int process_id,
+                               int thread_id,
+                               const ErrorInfo& info) OVERRIDE {
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+    ScopedVector<const Value> args;
+    args.push_back(new FundamentalValue(partition_id_));
+    args.push_back(new StringValue(base::Int64ToString(version_id)));
+    args.push_back(new FundamentalValue(process_id));
+    args.push_back(new FundamentalValue(thread_id));
+    scoped_ptr<DictionaryValue> value(new DictionaryValue());
+    value->SetString("message", info.error_message);
+    value->SetInteger("lineNumber", info.line_number);
+    value->SetInteger("columnNumber", info.column_number);
+    value->SetString("sourceURL", info.source_url.spec());
+    args.push_back(value.release());
+    web_ui_->CallJavascriptFunction("serviceworker.onErrorReported",
+                                    args.get());
+  }
+  int partition_id() const { return partition_id_; }
+
+ private:
+  const int partition_id_;
+  WebUI* const web_ui_;
+};
+
 ServiceWorkerInternalsUI::ServiceWorkerInternalsUI(WebUI* web_ui)
-    : WebUIController(web_ui) {
+    : WebUIController(web_ui), next_partition_id_(0) {
   WebUIDataSource* source =
       WebUIDataSource::Create(kChromeUIServiceWorkerInternalsHost);
   source->SetUseJsonJSFormatV2();
@@ -139,47 +205,45 @@ void ServiceWorkerInternalsUI::GetAllRegistrations(const ListValue* args) {
       base::Bind(&ServiceWorkerInternalsUI::AddContextFromStoragePartition,
                  base::Unretained(this));
   BrowserContext::ForEachStoragePartition(browser_context, add_context_cb);
-
-  BrowserContext::StoragePartitionCallback add_observer_cb =
-      base::Bind(&ServiceWorkerInternalsUI::AddObserverToStoragePartition,
-                 base::Unretained(this));
-  BrowserContext::ForEachStoragePartition(browser_context, add_observer_cb);
 }
 
 void ServiceWorkerInternalsUI::AddContextFromStoragePartition(
     StoragePartition* partition) {
+  int partition_id = 0;
   scoped_refptr<ServiceWorkerContextWrapper> context =
       static_cast<ServiceWorkerContextWrapper*>(
           partition->GetServiceWorkerContext());
+  if (PartitionObserver* observer =
+          observers_.get(reinterpret_cast<uintptr_t>(partition))) {
+    partition_id = observer->partition_id();
+  } else {
+    partition_id = next_partition_id_++;
+    scoped_ptr<PartitionObserver> new_observer(
+        new PartitionObserver(partition_id, web_ui()));
+    context->AddObserver(new_observer.get());
+    observers_.set(reinterpret_cast<uintptr_t>(partition), new_observer.Pass());
+  }
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
       base::Bind(
           &ServiceWorkerInternalsUI::OperationProxy::GetRegistrationsOnIOThread,
           new OperationProxy(AsWeakPtr(), scoped_ptr<ListValue>()),
+          partition_id,
           context,
           partition->GetPath()));
 }
 
-void ServiceWorkerInternalsUI::AddObserverToStoragePartition(
-    StoragePartition* partition) {
-  if (registered_partitions_.find(partition) != registered_partitions_.end())
-    return;
-  registered_partitions_.insert(partition);
-  scoped_refptr<ServiceWorkerContextWrapper> context =
-      static_cast<ServiceWorkerContextWrapper*>(
-          partition->GetServiceWorkerContext());
-  context->AddObserver(this);
-}
-
 void ServiceWorkerInternalsUI::RemoveObserverFromStoragePartition(
     StoragePartition* partition) {
-  if (registered_partitions_.find(partition) == registered_partitions_.end())
+  scoped_ptr<PartitionObserver> observer(
+      observers_.take_and_erase(reinterpret_cast<uintptr_t>(partition)));
+  if (!observer.get())
     return;
   scoped_refptr<ServiceWorkerContextWrapper> context =
       static_cast<ServiceWorkerContextWrapper*>(
           partition->GetServiceWorkerContext());
-  context->RemoveObserver(this);
+  context->RemoveObserver(observer.get());
 }
 
 namespace {
@@ -301,51 +365,8 @@ void ServiceWorkerInternalsUI::StopWorker(const ListValue* args) {
           scope));
 }
 
-void ServiceWorkerInternalsUI::OnWorkerStarted(int64 version_id,
-                                               int process_id,
-                                               int thread_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  web_ui()->CallJavascriptFunction("serviceworker.onWorkerStarted",
-                                   StringValue(base::Int64ToString(version_id)),
-                                   FundamentalValue(process_id),
-                                   FundamentalValue(thread_id));
-}
-
-void ServiceWorkerInternalsUI::OnWorkerStopped(int64 version_id,
-                                               int process_id,
-                                               int thread_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  web_ui()->CallJavascriptFunction("serviceworker.onWorkerStopped",
-                                   StringValue(base::Int64ToString(version_id)),
-                                   FundamentalValue(process_id),
-                                   FundamentalValue(thread_id));
-}
-
-void ServiceWorkerInternalsUI::OnVersionStateChanged(int64 version_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  web_ui()->CallJavascriptFunction(
-      "serviceworker.onVersionStateChanged",
-      StringValue(base::Int64ToString(version_id)));
-}
-
-void ServiceWorkerInternalsUI::OnErrorReported(int64 version_id,
-                                               int process_id,
-                                               int thread_id,
-                                               const ErrorInfo& info) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DictionaryValue value;
-  value.SetString("message", info.error_message);
-  value.SetInteger("lineNumber", info.line_number);
-  value.SetInteger("columnNumber", info.column_number);
-  value.SetString("sourceURL", info.source_url.spec());
-  web_ui()->CallJavascriptFunction("serviceworker.onErrorReported",
-                                   StringValue(base::Int64ToString(version_id)),
-                                   FundamentalValue(process_id),
-                                   FundamentalValue(thread_id),
-                                   value);
-}
-
 void ServiceWorkerInternalsUI::OperationProxy::GetRegistrationsOnIOThread(
+    int partition_id,
     ServiceWorkerContextWrapper* context,
     const base::FilePath& context_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -353,6 +374,7 @@ void ServiceWorkerInternalsUI::OperationProxy::GetRegistrationsOnIOThread(
   context->context()->storage()->GetAllRegistrations(
       base::Bind(&ServiceWorkerInternalsUI::OperationProxy::OnHaveRegistrations,
                  this,
+                 partition_id,
                  context_path));
 }
 
@@ -449,6 +471,7 @@ void UpdateVersionInfo(const ServiceWorkerVersionInfo& version,
 }  // namespace
 
 void ServiceWorkerInternalsUI::OperationProxy::OnHaveRegistrations(
+    int partition_id,
     const base::FilePath& context_path,
     const std::vector<ServiceWorkerRegistrationInfo>& registrations) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
@@ -458,6 +481,7 @@ void ServiceWorkerInternalsUI::OperationProxy::OnHaveRegistrations(
         base::Bind(
             &ServiceWorkerInternalsUI::OperationProxy::OnHaveRegistrations,
             this,
+            partition_id,
             context_path,
             registrations));
     return;
@@ -493,6 +517,7 @@ void ServiceWorkerInternalsUI::OperationProxy::OnHaveRegistrations(
     internals_->web_ui()->CallJavascriptFunction(
         "serviceworker.onPartitionData",
         result,
+        FundamentalValue(partition_id),
         StringValue(context_path.value()));
 }
 
diff --git a/content/browser/service_worker/service_worker_internals_ui.h b/content/browser/service_worker/service_worker_internals_ui.h
index 54d3c32..715fe70 100644
--- a/content/browser/service_worker/service_worker_internals_ui.h
+++ b/content/browser/service_worker/service_worker_internals_ui.h
@@ -7,6 +7,7 @@
 
 #include <set>
 
+#include "base/containers/scoped_ptr_hash_map.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ -29,31 +30,17 @@ class ServiceWorkerVersion;
 
 class ServiceWorkerInternalsUI
     : public WebUIController,
-      public ServiceWorkerContextObserver,
       public base::SupportsWeakPtr<ServiceWorkerInternalsUI> {
  public:
   explicit ServiceWorkerInternalsUI(WebUI* web_ui);
 
-  // ServiceWorkerContextObserver overrides:
-  virtual void OnWorkerStarted(int64 version_id,
-                               int process_id,
-                               int thread_id) OVERRIDE;
-  virtual void OnWorkerStopped(int64 version_id,
-                               int process_id,
-                               int thread_id) OVERRIDE;
-  virtual void OnVersionStateChanged(int64 version_id) OVERRIDE;
-  virtual void OnErrorReported(int64 version_id,
-                               int process_id,
-                               int thread_id,
-                               const ErrorInfo& info) OVERRIDE;
-
  private:
   class OperationProxy;
+  class PartitionObserver;
 
   virtual ~ServiceWorkerInternalsUI();
   void AddContextFromStoragePartition(StoragePartition* partition);
 
-  void AddObserverToStoragePartition(StoragePartition* partition);
   void RemoveObserverFromStoragePartition(StoragePartition* partition);
 
   // Called from Javascript.
@@ -68,7 +55,9 @@ class ServiceWorkerInternalsUI
       base::FilePath* partition_path,
       GURL* scope,
       scoped_refptr<ServiceWorkerContextWrapper>* context) const;
-  std::set<StoragePartition*> registered_partitions_;
+
+  base::ScopedPtrHashMap<uintptr_t, PartitionObserver> observers_;
+  int next_partition_id_;
 };
 
 }  // namespace content