perf_hooks: add class comments

nodejs · Apr 20, 2022 · fc94f0d · fc94f0d
1 parent 3127408
commit fc94f0d
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 22 deletions.
diff --git a/lib/internal/perf/performance.js b/lib/internal/perf/performance.js
@@ -166,6 +166,8 @@ ObjectDefineProperties(Performance.prototype, {
     enumerable: false,
     value: nodeTiming,
   },
+  // In the browser, this function is not public.  However, it must be used inside fetch
+  // which is a Node.js dependency, not a internal module
   markResourceTiming: {
     configurable: true,
     enumerable: false,

diff --git a/lib/internal/perf/resource_timing.js b/lib/internal/perf/resource_timing.js
@@ -13,13 +13,13 @@ const kTimingInfo = Symbol('kTimingInfo');
 const kInitiatorType = Symbol('kInitiatorType');
 
 class PerformanceResourceTiming extends InternalPerformanceEntry {
-  constructor(requestedUrl, start, initiatorType, timingInfo, cacheMode = '') {
-    super(requestedUrl, 'resource', start);
-    // https://w3c.github.io/resource-timing/#dfn-setup-the-resource-timing-entry
-    assert.ok(cacheMode === '' || cacheMode === 'local');
+  constructor(requestedUrl, initiatorType, timingInfo, cacheMode = '') {
+    super(requestedUrl, 'resource');
     this[kInitiatorType] = initiatorType;
     this[kRequestedUrl] = requestedUrl;
     // https://fetch.spec.whatwg.org/#fetch-timing-info
+    // This class is using timingInfo assuming it's already validated.
+    // The spec doesn't say to validate it in the class construction.
     this[kTimingInfo] = timingInfo;
     this[kCacheMode] = cacheMode;
   }
@@ -111,14 +111,27 @@ class PerformanceResourceTiming extends InternalPerformanceEntry {
 
   toJSON() {
     return {
-      initiatorType: this[kInitiatorType],
-      requestedUrl: this[kRequestedUrl],
-      cacheMode: this[kCacheMode],
-      // TODO(rafaelgss): should return timingInfo as well?
       name: this.name,
       entryType: this.entryType,
       startTime: this.startTime,
       duration: this.duration,
+      initiatorType: this[kInitiatorType],
+      nextHopProtocol: this.nextHopProtocol,
+      workerStart: this.workerStart,
+      redirectStart: this.redirectStart,
+      redirectEnd: this.redirectEnd,
+      fetchStart: this.fetchStart,
+      domainLookupStart: this.domainLookupStart,
+      domainLookupEnd: this.domainLookupEnd,
+      connectStart: this.connectStart,
+      connectEnd: this.connectEnd,
+      secureConnectionStart: this.secureConnectionStart,
+      requestStart: this.requestStart,
+      responseStart: this.responseStart,
+      responseEnd: this.responseEnd,
+      transferSize: this.transferSize,
+      encodedBodySize: this.encodedBodySize,
+      decodedBodySize: this.decodedBodySize,
     };
   }
 }
@@ -131,17 +144,10 @@ function markResourceTiming(
   global,
   cacheMode,
 ) {
-  // TODO(rafaelgss): Probably must have some asserts here for:
-  // - timingInfo
-  // - requestedUrl
-  // - initiatorType
-  // - cacheMode
-  // 1. Create a PerformanceResourceTiming object entry in global's realm.
-  // TODO(rafaelgss): why and how should I put `resource` into global object?
+  // https://w3c.github.io/resource-timing/#dfn-setup-the-resource-timing-entry
+  assert(cacheMode === '' || cacheMode === 'local');
   const resource = new PerformanceResourceTiming(
     requestedUrl,
-    // TODO(rafaelgss): not sure about that
-    timingInfo.startTime,
     initiatorType,
     timingInfo,
     cacheMode,

diff --git a/test/parallel/test-perf-hooks-resourcetiming.js b/test/parallel/test-perf-hooks-resourcetiming.js
@@ -142,13 +142,27 @@ function createTimingInfo({
   assert.strictEqual(resource.decodedBodySize, 0);
   assert.strictEqual(resource.transferSize, 0);
   assert.deepStrictEqual(resource.toJSON(), {
-    initiatorType,
-    requestedUrl,
-    cacheMode,
     name: requestedUrl,
     entryType: 'resource',
-    startTime: timingInfo.startTime,
-    duration: 0
+    startTime: 0,
+    duration: 0,
+    initiatorType,
+    nextHopProtocol: [],
+    workerStart: 0,
+    redirectStart: 0,
+    redirectEnd: 0,
+    fetchStart: 0,
+    domainLookupStart: 0,
+    domainLookupEnd: 0,
+    connectStart: 0,
+    connectEnd: 0,
+    secureConnectionStart: 0,
+    requestStart: 0,
+    responseStart: 0,
+    responseEnd: 0,
+    transferSize: 0,
+    encodedBodySize: 0,
+    decodedBodySize: 0,
   });
 
   assert(resource instanceof PerformanceEntry);