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

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

 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 281 matching lines @@
     String text =
         SerializeNodes<EditingStrategy>(accumulator, document, kIncludeNode);
 
     CString frame_html =
         document.Encoding().Encode(text, WTF::kEntitiesForUnencodables);
     resources_->push_back(SerializedResource(
         url, document.SuggestedMIMEType(),
         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())
       continue;
 
     Element& element = ToElement(*node);
     // We have to process in-line style as it might contain some resources
     // (typically background images).
     if (element.IsStyledElement()) {
       RetrieveResourcesForProperties(element.InlineStyle(), document);
       RetrieveResourcesForProperties(element.PresentationAttributeStyle(),
                                      document);
     }
 
     if (isHTMLImageElement(element)) {
       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)) {
       HTMLInputElement& input_element = toHTMLInputElement(element);
       if (input_element.type() == InputTypeNames::image &&
           input_element.ImageLoader()) {
         KURL url = input_element.Src();
         ImageResourceContent* cached_image =
             input_element.ImageLoader()->GetImage();
         AddImageToResources(cached_image, url);
       }
     } else if (isHTMLLinkElement(element)) {
       HTMLLinkElement& link_element = toHTMLLinkElement(element);
       if (CSSStyleSheet* sheet = link_element.sheet()) {
         KURL url = document.CompleteURL(
             link_element.getAttribute(HTMLNames::hrefAttr));
         SerializeCSSStyleSheet(*sheet, url);
       }
     } 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,
                                              const KURL& url) {
   // If the URL is invalid or if it is a data URL this means that this CSS is
   // defined inline, respectively in a <style> tag or in the data URL itself.
   bool is_inline_css = !url.IsValid() || url.ProtocolIsData();
   // If this CSS is not inline then it is identifiable by its URL. So just skip
   // it if it has already been analyzed before.
   if (!is_inline_css && (resource_urls_.Contains(url) ||
@@ skipping 225 matching lines @@
     emits_minus = ch == '-';
     builder.Append(ch);
   }
   CString escaped_url = builder.ToString().Ascii();
   return String::Format("saved from url=(%04d)%s",
                         static_cast<int>(escaped_url.length()),
                         escaped_url.data());
 }
 
 }  // namespace blink