Pre-work for adding usage counter for chrome.loadtimes

chrome/renderer/loadtimes_extension_bindings.cc

Index: chrome/renderer/loadtimes_extension_bindings.cc
diff --git a/chrome/renderer/loadtimes_extension_bindings.cc b/chrome/renderer/loadtimes_extension_bindings.cc
index a1aad263c03b96d47762d65ffce72580ab480ea0..ed6ad6ad6e6b9f3d88805867a0504f27b82185f3 100644
--- a/chrome/renderer/loadtimes_extension_bindings.cc
+++ b/chrome/renderer/loadtimes_extension_bindings.cc
@@ -8,6 +8,7 @@
 
 #include "base/time/time.h"
 #include "content/public/renderer/document_state.h"
+#include "extensions/renderer/v8_helpers.h"
 #include "net/http/http_response_info.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebPerformance.h"
@@ -105,6 +106,13 @@ class LoadTimesExtensionWrapper : public v8::Extension {
     return kTransitionOther;
   }
 
+  static void LoadtimesGetter(
+      v8::Local<v8::String> name,
+      const v8::PropertyCallbackInfo<v8::Value>& info) {
+    // TODO(panicker): Add Usage counter.
+    info.GetReturnValue().Set(info.Data());
+  }
+
   static void GetLoadTimes(const v8::FunctionCallbackInfo<v8::Value>& args) {
     args.GetReturnValue().SetNull();
     WebLocalFrame* frame = WebLocalFrame::frameForCurrentContext();
@@ -168,125 +176,106 @@ class LoadTimesExtensionWrapper : public v8::Extension {
     // referred to below this line, as JS setters below can invalidate these
     // pointers.
     v8::Isolate* isolate = args.GetIsolate();
-    v8::Local<v8::Context> ctx = isolate->GetCurrentContext();
-    v8::Local<v8::Object> load_times = v8::Object::New(isolate);
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate, "requestTime",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::Number::New(isolate, request_time))
-             .FromMaybe(false)) {
-      return;
-    }
 
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate, "startLoadTime",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::Number::New(isolate, start_load_time))
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate, "commitLoadTime",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::Number::New(isolate, commit_load_time))
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx,
-                   v8::String::NewFromUtf8(isolate, "finishDocumentLoadTime",
-                                           v8::NewStringType::kNormal)
-                       .ToLocalChecked(),
-                   v8::Number::New(isolate, finish_document_load_time))
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate, "finishLoadTime",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::Number::New(isolate, finish_load_time))
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate, "firstPaintTime",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::Number::New(isolate, first_paint_time))
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx,
-                   v8::String::NewFromUtf8(isolate, "firstPaintAfterLoadTime",
-                                           v8::NewStringType::kNormal)
-                       .ToLocalChecked(),
-                   v8::Number::New(isolate, first_paint_after_load_time))
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate, "navigationType",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::String::NewFromUtf8(isolate, navigation_type.c_str(),
-                                           v8::NewStringType::kNormal)
-                       .ToLocalChecked())
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate, "wasFetchedViaSpdy",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::Boolean::New(isolate, was_fetched_via_spdy))
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate, "wasNpnNegotiated",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::Boolean::New(isolate, was_npn_negotiated))
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx,
-                   v8::String::NewFromUtf8(isolate, "npnNegotiatedProtocol",
-                                           v8::NewStringType::kNormal)
-                       .ToLocalChecked(),
-                   v8::String::NewFromUtf8(isolate,
-                                           npn_negotiated_protocol.c_str(),
-                                           v8::NewStringType::kNormal)
-                       .ToLocalChecked())
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate,
-                                                "wasAlternateProtocolAvailable",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::Boolean::New(isolate, was_alternate_protocol_available))
-             .FromMaybe(false)) {
-      return;
-    }
-    if (!load_times
-             ->Set(ctx, v8::String::NewFromUtf8(isolate, "connectionInfo",
-                                                v8::NewStringType::kNormal)
-                            .ToLocalChecked(),
-                   v8::String::NewFromUtf8(isolate, connection_info.c_str(),
-                                           v8::NewStringType::kNormal)
-                       .ToLocalChecked())
-             .FromMaybe(false)) {
-      return;
-    }
-    args.GetReturnValue().Set(load_times);
+    v8::Local<v8::ObjectTemplate> load_times_tpl = v8::ObjectTemplate::New(
+        isolate);
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "requestTime",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Number::New(isolate, request_time));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "startLoadTime",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Number::New(isolate, start_load_time));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "commitLoadTime",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Number::New(isolate, commit_load_time));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "finishDocumentLoadTime",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Number::New(isolate, finish_document_load_time));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "finishLoadTime",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Number::New(isolate, finish_load_time));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "firstPaintTime",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Number::New(isolate, first_paint_time));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "firstPaintAfterLoadTime",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Number::New(isolate, first_paint_after_load_time));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "navigationType",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::String::NewFromUtf8(isolate, navigation_type.c_str(),
+                                v8::NewStringType::kNormal).ToLocalChecked());
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "wasFetchedViaSpdy",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Boolean::New(isolate, was_fetched_via_spdy));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "wasNpnNegotiated",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Boolean::New(isolate, was_npn_negotiated));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "npnNegotiatedProtocol",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::String::NewFromUtf8(isolate,
+                                npn_negotiated_protocol.c_str(),
+                                v8::NewStringType::kNormal).ToLocalChecked());
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "wasAlternateProtocolAvailable",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::Boolean::New(isolate, was_alternate_protocol_available));
+
+    load_times_tpl->SetNativeDataProperty(
+        v8::String::NewFromUtf8(isolate, "connectionInfo",
+                                v8::NewStringType::kNormal).ToLocalChecked(),
+        LoadtimesGetter,
+        nullptr,
+        v8::String::NewFromUtf8(isolate, connection_info.c_str(),
+                                v8::NewStringType::kNormal).ToLocalChecked());
+
+    args.GetReturnValue().Set(load_times_tpl->NewInstance());

[adamk, 2016/07/14 23:48:56]

So the only thing really bad about this approach is that V8 caches the instance
on the Context. Over time, this could lead to memory bloat, if loadTimes() is
called a lot.

The fact at this "just works" makes me think that perhaps we should just add a
"SetNativeDataProperty" to v8::Object. Adding jochen & verwaest to get their
thoughts on that idea.

[panicker, 2016/07/15 00:32:25]

Since loadtimes is applicable to a specific load, I'd expect it to be unusual to
call it multiple times per load in a script.
+Bryan to comment

[Bryan McQuade, 2016/07/18 18:33:55]

Yes, in code I've looked at, the common pattern is:

      var loadTimes = chrome.loadTimes;
      if (loadTimes) {
        ...
      }

   }
 
   static void GetCSI(const v8::FunctionCallbackInfo<v8::Value>& args) {