Metadata extraction for Copyless Paste

third_party/WebKit/Source/modules/copyless_paste/CopylessPasteExtractor.cpp

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "modules/copyless_paste/CopylessPasteExtractor.h"
+
+#include "core/HTMLNames.h"
+#include "core/dom/Document.h"
+#include "core/dom/ElementTraversal.h"
+#include "core/frame/LocalFrame.h"
+#include "core/html/HTMLElement.h"
+#include "platform/Histogram.h"
+#include "platform/instrumentation/tracing/TraceEvent.h"
+#include "wtf/text/StringBuilder.h"
+
+namespace blink {
+
+namespace {
+
+String extractMetadata(Element& root) {
+  StringBuilder result;
+  result.append("[");
+  bool multiple = false;
+  for (Element& element : ElementTraversal::descendantsOf(root)) {
+    if (!element.hasTagName(HTMLNames::scriptTag))

[esprehn, 2017/02/13 23:47:04]

This doesn't understand shadow DOM, also this is walking the entire document
which I'm not sure we want to do on every page load. How will this get called?

[wychen, 2017/02/14 00:26:08]

Not understanding shadow DOM should be fine, since the JSON-LD tag should never
be in one.

This does walk the entire document for main frames on every page load though. I
did think about the performance implication, and benchmarked this. On a typical
news page on N5X, CopylessPaste.ExtractionUs is 0.07ms, which is shorter than
WebCore.DistillabilityUs.

In some sense, this works similarly to DocumentStatisticsCollector. They both
walk the whole DOM tree, ignoring shadow DOM, but this one only gets called once
per page load.

+      continue;
+    if (element.hasTagName(HTMLNames::scriptTag) &&
+        element.hasAttribute(HTMLNames::typeAttr) &&

[esprehn, 2017/02/13 23:47:04]

no need to check hasAttribute, this is just doing duplicate work with the below
getAttribute. If it doesn't exist you'll get a nullAtom which won't be equal to
the string

[wychen, 2017/02/14 02:24:16]

Done.

+        element.getAttribute(HTMLNames::typeAttr) == "application/ld+json") {
+      if (multiple) {
+        result.append(",");
+      }
+      result.append(element.textContent(false));

[esprehn, 2017/02/13 23:47:04]

false is the default, remove this

[wychen, 2017/02/14 02:24:16]

Done.

+      multiple = true;
+    }
+  }
+  result.append("]");
+  return result.toString();
+}
+
+}  // namespace
+
+// Do JSON-LD extraction, which is equivalant to the following JavaScript code:
+//
+// var a=document.querySelectorAll("script[type=\"application/ld+json\"]");

[esprehn, 2017/02/13 23:47:04]

Can we store these in a map instead of scanning the entire document?

[wychen, 2017/02/14 00:26:08]

The JSON-LD data extracted here would be transient. After injecting to Icing, it
can be discarded.

Do you mean tightly integrating this extraction logic into the parser so that we
don't have to scan the DOM again? I thought about that, but parser can have
wider performance impact, while this only handles main frames, and is optional
(some embedders might not need this feature).

+// if(a) {
+//   var b=[];
+//   a.forEach(function(s) {b.push(JSON.parse(s.innerHTML)) });
+//   b;
+// }
+String CopylessPasteExtractor::extract(Document& document) {
+  TRACE_EVENT0("blink", "CopylessPasteExtractor::extract");
+
+  if (!document.frame() || !document.frame()->isMainFrame())
+    return "";
+
+  DCHECK(document.hasFinishedParsing());
+
+  Element* html = document.documentElement();
+  if (!html)
+    return "";

[esprehn, 2017/02/13 23:47:04]

emptyString

[wychen, 2017/02/14 02:24:16]

Done.

+
+  double startTime = monotonicallyIncreasingTime();
+
+  // Traverse the DOM tree and extract the metadata.
+  String result = extractMetadata(*html);
+
+  double elapsedTime = monotonicallyIncreasingTime() - startTime;
+
+  DEFINE_STATIC_LOCAL(CustomCountHistogram, extractionHistogram,
+                      ("CopylessPaste.ExtractionUs", 1, 1000000, 50));
+  extractionHistogram.count(static_cast<int>(1e6 * elapsedTime));
+  return result;
+}
+
+}  // namespace blink