[Offline Pages] Adding missing image/CSS detection in FrameSerializer.

third_party/WebKit/Source/core/frame/FrameSerializer.cpp

Index: third_party/WebKit/Source/core/frame/FrameSerializer.cpp
diff --git a/third_party/WebKit/Source/core/frame/FrameSerializer.cpp b/third_party/WebKit/Source/core/frame/FrameSerializer.cpp
index a2258654313ad94f65350fda26b7e48f38c62b14..c743acab4916e42e68a2409d6d99463216a78af2 100644
--- a/third_party/WebKit/Source/core/frame/FrameSerializer.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameSerializer.cpp
@@ -299,6 +299,12 @@ void FrameSerializer::SerializeFrame(const LocalFrame& frame) {
         SharedBuffer::Create(frame_html.data(), frame_html.length())));
   }
 
+  int total_image_count = 0;
+  int missing_image_count = 0;
+  int total_css_count = 0;
+  int missing_css_count = 0;
+  bool should_collect_problem_metric =
+      delegate_.ShouldCollectProblemMetric() && frame.IsMainFrame();
   for (Node* node : serialized_nodes) {
     DCHECK(node);
     if (!node->IsElementNode())
@@ -317,6 +323,11 @@ void FrameSerializer::SerializeFrame(const LocalFrame& frame) {
       HTMLImageElement& image_element = toHTMLImageElement(element);
       KURL url =
           document.CompleteURL(image_element.getAttribute(HTMLNames::srcAttr));
+      if (should_collect_problem_metric) {
+        total_image_count++;
+        if (!image_element.complete())
+          missing_image_count++;

[Pete Williamson, 2017/06/06 22:27:25]

I think it is better to count requested vs completed images, as opposed to total
and missing - the concepts are a bit more concrete, and should make maintenance
a bit easier.  Same comment for CSS below.

+      }
       ImageResourceContent* cached_image = image_element.CachedImage();
       AddImageToResources(cached_image, url);
     } else if (isHTMLInputElement(element)) {
@@ -337,10 +348,33 @@ void FrameSerializer::SerializeFrame(const LocalFrame& frame) {
       }
     } else if (isHTMLStyleElement(element)) {
       HTMLStyleElement& style_element = toHTMLStyleElement(element);
-      if (CSSStyleSheet* sheet = style_element.sheet())
+      CSSStyleSheet* sheet = style_element.sheet();
+      if (sheet)
         SerializeCSSStyleSheet(*sheet, KURL());
+      if (should_collect_problem_metric) {
+        total_css_count++;
+        if (sheet && sheet->IsLoading())
+          missing_css_count++;
+      }
     }
   }
+  if (should_collect_problem_metric) {
+    // Report detectors through UMA.
+    DCHECK_LE(missing_image_count, total_image_count);
+    UMA_HISTOGRAM_PERCENTAGE(
+        "PageSerialization.ProblemDetection.MissingImagePercentage",

[Pete Williamson, 2017/06/06 22:27:25]

I think that stating this in the positive will be slightly easier to understand
and maintain - CompletedImagePercentage vs MissingImagePercentage.  Then the
calculation would be (completed_images/requested_images) * 100

Same for CSS.

+        static_cast<int64_t>(missing_image_count * 100 / total_image_count));
+    UMA_HISTOGRAM_COUNTS_100(

[Pete Williamson, 2017/06/06 22:27:25]

Is 100 buckets too granular?  Maybe about 20 buckets, each 5% would be easier to
reason about, and easier to read in the UMA histogram data.  One bucket should
be exactly 0 completion%, one exactly 100% completion, and others in between
running about 5% of the completion range each.

+        "PageSerialization.ProblemDetection.TotalImageCount",
+        static_cast<int64_t>(total_image_count));
+
+    DCHECK_LE(missing_css_count, total_css_count);
+    UMA_HISTOGRAM_PERCENTAGE(
+        "PageSerialization.ProblemDetection.MissingCSSPercentage",
+        static_cast<int64_t>(missing_css_count * 100 / total_css_count));
+    UMA_HISTOGRAM_COUNTS_100("PageSerialization.ProblemDetection.TotalCSSCount",
+                             static_cast<int64_t>(total_css_count));
+  }
 }
 
 void FrameSerializer::SerializeCSSStyleSheet(CSSStyleSheet& style_sheet,