Add UMA histograms to MHTML save operations.

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 41 matching lines @@
 #include "core/frame/LocalFrame.h"
 #include "core/html/HTMLFrameElementBase.h"
 #include "core/html/HTMLImageElement.h"
 #include "core/html/HTMLInputElement.h"
 #include "core/html/HTMLLinkElement.h"
 #include "core/html/HTMLMetaElement.h"
 #include "core/html/HTMLStyleElement.h"
 #include "core/html/ImageDocument.h"
 #include "core/style/StyleFetchedImage.h"
 #include "core/style/StyleImage.h"
+#include "platform/Histogram.h"
 #include "platform/SerializedResource.h"
 #include "platform/TraceEvent.h"
 #include "platform/graphics/Image.h"
 #include "platform/heap/Handle.h"
 #include "wtf/HashSet.h"
 #include "wtf/text/CString.h"
 #include "wtf/text/StringBuilder.h"
 #include "wtf/text/TextEncoding.h"
 #include "wtf/text/WTFString.h"
 
@@ skipping 164 matching lines @@
 // documents which leads to bugs like <https://crbug.com/251898>. Not being
 // able to rewrite URLs inside CSS documents means that resources imported from
 // url(...) statements in CSS might not work when rewriting links for the
 // "Webpage, Complete" method of saving a page. It will take some work but it
 // needs to be done if we want to continue to support non-MHTML saved pages.
 
 FrameSerializer::FrameSerializer(
     Vector<SerializedResource>& resources,
     Delegate& delegate)
     : m_resources(&resources)
+    , m_isSerializingCss(false)
     , m_delegate(delegate)
 {
 }
 
 void FrameSerializer::serializeFrame(const LocalFrame& frame)
 {
     TRACE_EVENT0("page-serialization", "FrameSerializer::serializeFrame");
     ASSERT(frame.document());
     Document& document = *frame.document();
     KURL url = document.url();
 
     // If frame is an image document, add the image and don't continue
     if (document.isImageDocument()) {
         ImageDocument& imageDocument = toImageDocument(document);
         addImageToResources(imageDocument.cachedImage(), url);
         return;
     }
 
-    TRACE_EVENT_BEGIN0("page-serialization", "FrameSerializer::serializeFrame HTML");
     HeapVector<Member<Node>> serializedNodes;
-    SerializerMarkupAccumulator accumulator(m_delegate, document, serializedNodes);
-    String text = serializeNodes<EditingStrategy>(accumulator, document, IncludeNode);
+    {
+        TRACE_EVENT0("page-serialization", "FrameSerializer::serializeFrame HTML");
+        SCOPED_BLINK_UMA_HISTOGRAM_TIMER("PageSerialization.HtmlSerializationTime");
+        SerializerMarkupAccumulator accumulator(m_delegate, document, serializedNodes);
+        String text = serializeNodes<EditingStrategy>(accumulator, document, IncludeNode);
 
-    CString frameHTML = document.encoding().encode(text, WTF::EntitiesForUnencodables);
-    m_resources->append(SerializedResource(url, document.suggestedMIMEType(), SharedBuffer::create(frameHTML.data(), frameHTML.length())));
-    TRACE_EVENT_END0("page-serialization", "FrameSerializer::serializeFrame HTML");
+        CString frameHTML = document.encoding().encode(text, WTF::EntitiesForUnencodables);
+        m_resources->append(SerializedResource(url, document.suggestedMIMEType(), SharedBuffer::create(frameHTML.data(), frameHTML.length())));
+    }
 
     for (Node* node: serializedNodes) {
         ASSERT(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);
@@ skipping 23 matching lines @@
             if (CSSStyleSheet* sheet = styleElement.sheet())
                 serializeCSSStyleSheet(*sheet, KURL());
         }
     }
 }
 
 void FrameSerializer::serializeCSSStyleSheet(CSSStyleSheet& styleSheet, const KURL& url)
 {
     TRACE_EVENT2("page-serialization", "FrameSerializer::serializeCSSStyleSheet",
         "type", "CSS", "url", url.elidedString().utf8().data());
+    // Only report UMA metric if this is not a reentrant CSS serialization call.
+    double cssStartTime = 0;
+    if (!m_isSerializingCss) {
+        m_isSerializingCss = true;
+        cssStartTime = WTF::monotonicallyIncreasingTime();
+    }
+
     StringBuilder cssText;
     cssText.append("@charset \"");
     cssText.append(styleSheet.contents()->charset().lower());
     cssText.append("\";\n\n");
 
     for (unsigned i = 0; i < styleSheet.length(); ++i) {
         CSSRule* rule = styleSheet.item(i);
         String itemText = rule->cssText();
         if (!itemText.isEmpty()) {
             cssText.append(itemText);
             if (i < styleSheet.length() - 1)
                 cssText.append("\n\n");
         }
 
         // Some rules have resources associated with them that we need to retrieve.
         serializeCSSRule(rule);
     }
 
     if (shouldAddURL(url)) {
         WTF::TextEncoding textEncoding(styleSheet.contents()->charset());
         ASSERT(textEncoding.isValid());
         String textString = cssText.toString();
         CString text = textEncoding.encode(textString, WTF::CSSEncodedEntitiesForUnencodables);
         m_resources->append(SerializedResource(url, String("text/css"), SharedBuffer::create(text.data(), text.length())));
         m_resourceURLs.add(url);
     }
+
+    if (cssStartTime != 0) {
+        m_isSerializingCss = false;
+        DEFINE_STATIC_LOCAL(CustomCountHistogram, cssHistogram, ("PageSerialization.CSSSerializationTime", 0, 10000000, 50));
+        cssHistogram.count(static_cast<int64_t>((WTF::monotonicallyIncreasingTime() - cssStartTime) * 1000000));
+    }
 }
 
 void FrameSerializer::serializeCSSRule(CSSRule* rule)
 {
     ASSERT(rule->parentStyleSheet()->ownerDocument());
     Document& document = *rule->parentStyleSheet()->ownerDocument();
 
     switch (rule->type()) {
     case CSSRule::kStyleRule:
         retrieveResourcesForProperties(&toCSSStyleRule(rule)->styleRule()->properties(), document);
@@ skipping 56 matching lines @@
     m_resourceURLs.add(url);
 }
 
 void FrameSerializer::addImageToResources(ImageResource* image, const KURL& url)
 {
     if (!image || !image->hasImage() || image->errorOccurred() || !shouldAddURL(url))
         return;
 
     TRACE_EVENT2("page-serialization", "FrameSerializer::addImageToResources",
         "type", "image", "url", url.elidedString().utf8().data());
+    double imageStartTime = WTF::monotonicallyIncreasingTime();
+
     RefPtr<const SharedBuffer> data = image->getImage()->data();
     addToResources(*image, data, url);
+
+    // If we're already reporting time for CSS serialization don't report it for
+    // this image to avoid reporting the same time twice.
+    if (!m_isSerializingCss) {
+        DEFINE_STATIC_LOCAL(CustomCountHistogram, imageHistogram, ("PageSerialization.ImageSerializationTime", 0, 10000000, 50));
+        imageHistogram.count(static_cast<int64_t>((WTF::monotonicallyIncreasingTime() - imageStartTime) * 1000000));

[Ilya Sherman, 2016/09/24 02:53:51]

nit: Are there any utility functions in Blink that provide more readable
conversions among time units than just multiplying by a really large number?  If
not, it might be nice to add such -- it's hard to count the number of zeros here
(and above, too, actually).

[carlosk, 2016/09/27 21:07:29]

I'm not very used to Blink's codebase but I think the answer is no, there's no
constants like that. At least that's what searching for the use pattern of these
histograms shows me [1]. Hopefully at some point we will use the histogram
macros from base/ now that the repos are merged.

I agree that is annoying and I found a way to making it somewhat easier to read:
using the scientific notation as it's the case in a few hits of the referenced
codesearch. WDYT?

[1] https://cs.chromium.org/search/?q="CustomCountHistogram"

[Ilya Sherman, 2016/09/28 00:42:15]

Yep, it's a bit nicer this way -- thanks =)

+    }
 }
 
 void FrameSerializer::addFontToResources(FontResource* font)
 {
     if (!font || !font->isLoaded() || !font->resourceBuffer() || !shouldAddURL(font->url()))
         return;
 
     RefPtr<const SharedBuffer> data(font->resourceBuffer());
 
     addToResources(*font, data, font->url());
@@ skipping 55 matching lines @@
             continue;
         }
         emitsMinus = ch == '-';
         builder.append(ch);
     }
     CString escapedUrl = builder.toString().ascii();
     return String::format("saved from url=(%04d)%s", static_cast<int>(escapedUrl.length()), escapedUrl.data());
 }
 
 } // namespace blink