Randomize order in which ready component updates are applied. On demand updates

chrome/browser/component_updater/component_updater_service.cc

Index: chrome/browser/component_updater/component_updater_service.cc
diff --git a/chrome/browser/component_updater/component_updater_service.cc b/chrome/browser/component_updater/component_updater_service.cc
index 2095c0593a6ac4a2a91f7b5795a35a61249f920a..b032a53c8476cc0f81562dbad6c3d480bff3008d 100644
--- a/chrome/browser/component_updater/component_updater_service.cc
+++ b/chrome/browser/component_updater/component_updater_service.cc
@@ -18,6 +18,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
+#include "base/strings/stringprintf.h"

[Sorin Jianu, 2014/02/11 22:10:23]

What is the reason for including this header?

[bartekn, 2014/02/13 22:51:30]

Whoops, sorry, I used it for printing some debug messages, that I removed, but
forgot to remove the include.

 #include "base/threading/sequenced_worker_pool.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/browser_process.h"
@@ -574,23 +575,29 @@ void CrxUpdateService::ProcessPendingItems() {
 }
 
 CrxUpdateItem* CrxUpdateService::FindReadyComponent() const {
-  class Helper {
-   public:
-    static bool IsReadyOnDemand(CrxUpdateItem* item) {
-      return item->on_demand && IsReady(item);
-    }
-    static bool IsReady(CrxUpdateItem* item) {
-      return item->status == CrxUpdateItem::kCanUpdate;
+  std::vector<CrxUpdateItem*> ready;

[Sorin Jianu, 2014/02/11 22:10:23]

One idea to filter the work_items_ is by using std::copy_if and eventually a
helper function that takes a predicate as an argument and returns a vector, as
in:

std::vector<CrxUpdateItem*> ready_items(Helper::FilterWorkItems(IsReady));

[bartekn, 2014/02/13 22:51:30]

I tried it, but for some reason copy_if doesn't seem to exist. It says that
copy_if isn't in std, despite I include <algorithm>

+  std::vector<CrxUpdateItem*> ready_on_demand;
+  for (std::vector<CrxUpdateItem*>::const_iterator it = work_items_.begin();
+       it != work_items_.end();
+       ++it) {
+    CrxUpdateItem* item = *it;
+    if (item->status == CrxUpdateItem::kCanUpdate) {
+      if (item->on_demand) {

[Sorin Jianu, 2014/02/11 22:10:23]

the chrome style guide prefers not fully bracing statements such as the inner if
in this case.

[bartekn, 2014/02/13 22:51:30]

Done.

+        ready_on_demand.push_back(item);
+      } else {
+        ready.push_back(item);
+      }
     }
-  };
+  }
 
-  std::vector<CrxUpdateItem*>::const_iterator it = std::find_if(
-      work_items_.begin(), work_items_.end(), Helper::IsReadyOnDemand);
-  if (it != work_items_.end())
-    return *it;
-  it = std::find_if(work_items_.begin(), work_items_.end(), Helper::IsReady);
-  if (it != work_items_.end())
-    return *it;
+  if (ready_on_demand.size() > 0) {
+    int r = rand();

[Sorin Jianu, 2014/02/11 22:10:23]

I suggest using a function from base/rand_util.h and also paying attention to
how long the types are.

[bartekn, 2014/02/13 22:51:30]

Done.

+    return ready_on_demand[r % ready_on_demand.size()];
+  }
+  if (ready.size() > 0) {
+    int r = rand();
+    return ready[r % ready.size()];
+  }
   return NULL;
 }