Plumb Task Scheduler Histograms to the Task Scheduler Internals Page

chrome/browser/ui/webui/task_scheduler_internals/task_scheduler_internals_ui.cc

Index: chrome/browser/ui/webui/task_scheduler_internals/task_scheduler_internals_ui.cc
diff --git a/chrome/browser/ui/webui/task_scheduler_internals/task_scheduler_internals_ui.cc b/chrome/browser/ui/webui/task_scheduler_internals/task_scheduler_internals_ui.cc
index afec2204a08d08634de7a3d881bde8ec2051c505..ad6e906d6901c4938e53d3cd4026db73fc74e10b 100644
--- a/chrome/browser/ui/webui/task_scheduler_internals/task_scheduler_internals_ui.cc
+++ b/chrome/browser/ui/webui/task_scheduler_internals/task_scheduler_internals_ui.cc
@@ -4,22 +4,105 @@
 
 #include "chrome/browser/ui/webui/task_scheduler_internals/task_scheduler_internals_ui.h"
 
+#include <utility>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"

[fdoray, 2016/10/14 20:59:06]

Gab told me that including bind_helpers.h is not required
https://codereview.chromium.org/2180953002/diff/1/base/task_runner.cc#newcode8

[robliao, 2016/10/17 22:36:57]

Oddly enough, it's even further than that.
bind.h -> bind_internal.h -> bind_helpers.h.

The guidance from the style guide is
If you rely on symbols from bar.h, don't count on the fact that you included
foo.h which (currently) includes bar.h: include bar.h yourself, unless foo.h
explicitly demonstrates its intent to provide you the symbols of bar.h.

[gab, 2016/10/18 14:28:48]

I think it does demonstrate its intent by having bind.h refer to callback.md
which documents to use the bind helpers without mentioning the need for an extra
include.

I'm a big fan of IWUU but in this case I think bind.h is known to come with
everything you need to use it (i.e. it would be silly for every callsite to
include both bind.h and bind_helpers.h).

+#include "base/memory/ptr_util.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/histogram_samples.h"
 #include "base/task_scheduler/task_scheduler.h"
+#include "base/values.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/task_scheduler_internals_resources.h"
 #include "content/public/browser/web_ui_data_source.h"
+#include "content/public/browser/web_ui_message_handler.h"
+
+namespace {
+
+std::unique_ptr<base::Value> SnapshotHistogramToValue(
+    const base::HistogramBase* histogram) {
+  std::unique_ptr<base::ListValue> values =
+      base::MakeUnique<base::ListValue>();
+
+  std::unique_ptr<base::HistogramSamples> samples =
+      histogram->SnapshotSamples();
+  std::unique_ptr<base::SampleCountIterator> iterator = samples->Iterator();
+  while (!iterator->Done()) {
+    base::HistogramBase::Sample min;
+    base::HistogramBase::Sample max;
+    base::HistogramBase::Count count;
+    iterator->Get(&min, &max, &count);
+
+    std::unique_ptr<base::DictionaryValue> bucket =
+        base::MakeUnique<base::DictionaryValue>();
+    bucket->SetInteger("min", min);
+    bucket->SetInteger("max", max);
+    bucket->SetInteger("count", count);
+
+    values->Append(std::move(bucket));
+    iterator->Next();
+  }
+  return std::move(values);

[fdoray, 2016/10/14 20:59:06]

no move required when returning a local variable 

https://www.chromium.org/rvalue-references#TOC-11.-Don-t-return-references.-D....

http://stackoverflow.com/a/14856553

[robliao, 2016/10/17 22:36:57]

Done.

[robliao, 2016/10/20 00:01:03]

It appears this is actually needed!
https://www.chromium.org/rvalue-references#TOC-11.-Don-t-return-references.-D....

There's an exception for returning derived types below.

+}
+
+class TaskSchedulerDataHandler : public content::WebUIMessageHandler {
+ public:
+  TaskSchedulerDataHandler() = default;
+
+  // content::WebUIMessageHandler:
+  void RegisterMessages() override {
+    web_ui()->RegisterMessageCallback(
+        "getTaskSchedulerData",
+        base::Bind(&TaskSchedulerDataHandler::GetTaskSchedulerData,
+                   base::Unretained(this)));
+  }
+
+ private:
+  void GetTaskSchedulerData(const base::ListValue*) {
+    base::DictionaryValue data;
+
+    base::TaskScheduler* task_scheduler = base::TaskScheduler::GetInstance();
+    data.SetBoolean("instantiated", !!task_scheduler);
+
+    if (task_scheduler) {
+      std::unique_ptr<base::ListValue> histogram_value =
+          base::MakeUnique<base::ListValue>();
+      std::vector<const base::HistogramBase*> histograms =
+          task_scheduler->GetHistograms();
+
+      for (const base::HistogramBase* const histogram : histograms) {
+        std::unique_ptr<base::DictionaryValue> buckets =
+            base::MakeUnique<base::DictionaryValue>();
+        buckets->SetString("name", histogram->histogram_name());
+        buckets->Set("buckets", SnapshotHistogramToValue(histogram));
+        histogram_value->Append(std::move(buckets));
+      }
+
+      data.Set("histograms", std::move(histogram_value));
+    }
+
+    web_ui()->CallJavascriptFunctionUnsafe(
+        "TaskSchedulerInternals.onGetTaskSchedulerData", data);
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(TaskSchedulerDataHandler);
+};
+
+}  // namespace
 
 TaskSchedulerInternalsUI::TaskSchedulerInternalsUI(content::WebUI* web_ui)
     : content::WebUIController(web_ui) {
+  web_ui->AddMessageHandler(new TaskSchedulerDataHandler());
+
   content::WebUIDataSource* html_source =
       content::WebUIDataSource::Create(
           chrome::kChromeUITaskSchedulerInternalsHost);
-
-  html_source->AddString("status", base::TaskScheduler::GetInstance()
-      ? "Instantiated"
-      : "Not Instantiated");
-
+  html_source->AddResourcePath(
+      "index.css", IDR_TASK_SCHEDULER_INTERNALS_RESOURCES_INDEX_CSS);
+  html_source->AddResourcePath(
+      "index.js", IDR_TASK_SCHEDULER_INTERNALS_RESOURCES_INDEX_JS);
   html_source->SetDefaultResource(
       IDR_TASK_SCHEDULER_INTERNALS_RESOURCES_INDEX_HTML);