Lower the priority of shared workers

content/browser/worker_host/worker_service_impl.cc

Index: content/browser/worker_host/worker_service_impl.cc
diff --git a/content/browser/worker_host/worker_service_impl.cc b/content/browser/worker_host/worker_service_impl.cc
index 703b31f02ce3b7f2df5423c95c9c37b528c1a923..6c8ce8bd87d6299cbdbb4f32c7ace80ce51a90e3 100644
--- a/content/browser/worker_host/worker_service_impl.cc
+++ b/content/browser/worker_host/worker_service_impl.cc
@@ -10,12 +10,20 @@
 #include "base/logging.h"
 #include "base/threading/thread.h"
 #include "content/browser/devtools/worker_devtools_manager.h"
+#include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/worker_host/worker_message_filter.h"
 #include "content/browser/worker_host/worker_process_host.h"
 #include "content/common/view_messages.h"
 #include "content/common/worker_messages.h"
 #include "content/public/browser/child_process_data.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/resource_context.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/browser/worker_service_observer.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/process_type.h"
@@ -34,7 +42,12 @@ WorkerServiceImpl* WorkerServiceImpl::GetInstance() {
   return Singleton<WorkerServiceImpl>::get();
 }
 
-WorkerServiceImpl::WorkerServiceImpl() : next_worker_route_id_(0) {
+WorkerServiceImpl::WorkerServiceImpl()
+    : next_worker_route_id_(0) {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&WorkerServiceImpl::RegisterObserver,
+                 base::Unretained(this)));
 }
 
 WorkerServiceImpl::~WorkerServiceImpl() {
@@ -457,6 +470,10 @@ void WorkerServiceImpl::NotifyWorkerDestroyed(
                     WorkerDestroyed(process->GetData().id, worker_route_id));
 }
 
+void WorkerServiceImpl::NotifyWorkerProcessCreated() {
+  PostTaskToGatherAndUpdateWorkerPriorities();
+}
+
 WorkerProcessHost::WorkerInstance* WorkerServiceImpl::FindSharedWorkerInstance(
     const GURL& url,
     const string16& name,
@@ -526,4 +543,126 @@ WorkerProcessHost::WorkerInstance* WorkerServiceImpl::CreatePendingInstance(
   return &pending_shared_workers_.back();
 }
 
+void WorkerServiceImpl::RegisterObserver() {

[jam, 2013/05/09 18:02:57]

this class is currently completely on the IO thread. adding members and methods
that are used on the UI thread will make it harder to read and possibly
introduce bugs. two things that jump at me off the top of my head:
-now registrar_'s destructor would run on the IO thread, which would
assert/crash since it registered on a different thread
-the base::bind that uses Unretained in the constructor might crash in unittests
which create this object and delete it before the task run. with
https://codereview.chromium.org/8947021/ which will land soon, singletons will
be deleted between each unit test 

how about having an inner ref counted class that does the work on the UI thread,
and notifies the WorkerServiceImpl object on the UI thread?

[shatch, 2013/05/09 22:42:49]

Done.

+  registrar_.Add(this, content::NOTIFICATION_RENDER_WIDGET_VISIBILITY_CHANGED,

[jam, 2013/05/09 18:02:57]

nit: likewise, here and below remove the "content::"

[shatch, 2013/05/09 22:42:49]

Done.

+                 content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+}
+
+void WorkerServiceImpl::UpdateWorkerPrioritiesFromVisibleSet(
+    const std::set<std::pair<int, int> >* visible_renderer_ids) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  for (WorkerProcessHostIterator iter; !iter.Done(); ++iter) {
+    if (!iter->IsProcessLaunched())
+      continue;
+    bool throttle = true;
+
+    for (WorkerProcessHost::Instances::const_iterator instance =
+        iter->instances().begin(); instance != iter->instances().end();
+        ++instance) {
+
+      // This code assumes one worker per process
+      WorkerProcessHost::Instances::const_iterator first_instance =
+          iter->instances().begin();
+      if (first_instance == iter->instances().end())
+        continue;
+
+      WorkerDocumentSet::DocumentInfoSet::const_iterator info =
+          first_instance->worker_document_set()->documents().begin();
+
+      for (; info != first_instance->worker_document_set()->documents().end();
+          ++info) {
+        std::pair<int, int> id(
+            info->render_process_id(), info->render_view_id());
+        if (visible_renderer_ids->find(id) != visible_renderer_ids->end()) {
+          throttle = false;
+          break;
+        }
+      }
+
+      if (!throttle ) {
+        break;
+      }
+    }
+
+    iter->SetBackgrounded(throttle);
+  }
+}
+
+void WorkerServiceImpl::OnRenderWidgetVisibilityChanged(
+    std::pair<int, int> id) {
+  std::set<std::pair<int, int> > visible_renderer_ids;
+
+  visible_renderer_ids.insert(id);
+
+  UpdateWorkerPrioritiesFromVisibleSet(&visible_renderer_ids);
+}
+
+void WorkerServiceImpl::GatherVisibleIDsAndUpdateWorkerPriorities() {
+  std::set<std::pair<int, int> >* visible_renderer_ids =
+      new std::set<std::pair<int, int> >();
+
+  // Gather up all the visible renderer process/view pairs
+  for (content::RenderProcessHost::iterator it =
+      content::RenderProcessHost::AllHostsIterator();

[darin (slow to review), 2013/05/10 23:42:13]

nit: like function calls, we usually wrap the line starting the column after the
"("... which is a 5 space indent in the case of for loops.

[shatch, 2013/05/13 17:47:47]

Done.

+      !it.IsAtEnd(); it.Advance()) {
+    content::RenderProcessHost* host = it.GetCurrentValue();
+    if (host->VisibleWidgetCount()) {
+      for (content::RenderProcessHost::RenderWidgetHostsIterator rit =
+          host->GetRenderWidgetHostsIterator(); !rit.IsAtEnd(); rit.Advance()) {
+        content::RenderWidgetHost* widget = host->GetRenderWidgetHostByID(
+            rit.GetCurrentKey());
+        if (widget) {
+          content::RenderWidgetHostView* view = widget->GetView();
+          if (view && view->IsShowing()) {
+            visible_renderer_ids->insert(
+                std::pair<int, int>(host->GetID(),
+                                    widget->GetRoutingID()));
+          }
+        }
+      }
+    }
+  }
+
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&WorkerServiceImpl::UpdateWorkerPrioritiesFromVisibleSet,
+                 base::Unretained(this),
+                 base::Owned(visible_renderer_ids)));
+}
+
+void WorkerServiceImpl::PostTaskToGatherAndUpdateWorkerPriorities() {
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(
+          &WorkerServiceImpl::GatherVisibleIDsAndUpdateWorkerPriorities,
+          base::Unretained(this)));
+}
+
+void WorkerServiceImpl::Observe(int type,
+                                const NotificationSource& source,
+                                const NotificationDetails& details) {
+  if (type == content::NOTIFICATION_RENDER_WIDGET_VISIBILITY_CHANGED) {
+    bool visible = *content::Details<bool>(details).ptr();
+
+    if (visible) {
+      int id = content::Source<content::RenderWidgetHost>(source).ptr()->

[jam, 2013/05/09 18:02:57]

nit: naming wise, convention is
render_view_id
render_process_id
please use this convention because it makes it clear what the IDs are referring
to
(also in other places in this file)

[shatch, 2013/05/09 22:42:49]

Done.

+          GetRoutingID();
+      int pid = content::Source<content::RenderWidgetHost>(source).ptr()->
+          GetProcess()->GetID();
+
+      BrowserThread::PostTask(
+          BrowserThread::IO, FROM_HERE,
+          base::Bind(&WorkerServiceImpl::OnRenderWidgetVisibilityChanged,
+                     base::Unretained(this),
+                     std::pair<int, int>(pid, id)));
+    }
+  }
+  else if (type == NOTIFICATION_RENDERER_PROCESS_CREATED) {
+    PostTaskToGatherAndUpdateWorkerPriorities();
+  }
+}
+
 }  // namespace content