Discard hidden articles when using fast path

java/org/chromium/distiller/DomUtil.java

 // Copyright 2014 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.
 
 package org.chromium.distiller;
 
 import com.google.gwt.core.client.JsArray;
 import com.google.gwt.core.client.JsArrayString;
 import com.google.gwt.dom.client.AnchorElement;
 import com.google.gwt.dom.client.Document;
@@ skipping 84 matching lines @@
     }-*/;
 
     public static boolean isVisible(Element e) {
         Style style = getComputedStyle(e);
         double opacity = JavaScript.parseFloat(style.getOpacity());
         return !(style.getDisplay().equals("none") ||
                 style.getVisibility().equals("hidden") ||
                 opacity == 0.0F);
     }
 
+    /**
+     * Verifies if a given element is visible by checking its offset.
+     */
+    public static boolean isVisibleByItsOffset(Element e) {
+        return !(e.getOffsetHeight() <= 0 || e.getOffsetWidth() <= 0);

[wychen, 2015/12/10 23:33:36]

I amended the current isVisible() implementation in this CL:
https://codereview.chromium.org/1508963003/

You might want to rebase on top of that.

[wychen, 2016/06/01 08:46:30]

Let's just use the logic from there without rebasing for now.

+    }
+
+    /**
+     * Get the element of the main article, if any.
+     * @return An element of article (not necessarily the html5 article element).
+     */
+    public static Element getArticleElement(Element root) {
+        NodeList<Element> allArticles = root.getElementsByTagName("ARTICLE");
+        List<Element> visibleElements = getVisibleElements(allArticles);
+        // Having multiple article elements usually indicates a bad case for this shortcut.
+        // TODO(wychen): some sites exclude things like title and author in article element.
+        if (visibleElements.size() == 1) {
+            return visibleElements.get(0);
+        }
+        // Note that the CSS property matching is case sensitive, and "Article" is the correct
+        // capitalization.
+        String query = "[itemscope][itemtype*=\"Article\"],[itemscope][itemtype*=\"Post\"]";
+        allArticles = DomUtil.querySelectorAll(root, query);
+        visibleElements = getVisibleElements(allArticles);
+        // It is commonly seen that the article is wrapped separately or in multiple layers.
+        if (visibleElements.size() > 0) {
+            return Element.as(DomUtil.getNearestCommonAncestor(visibleElements));
+        }
+        return null;
+    }
+
+    /**
+     * Get a list of visible elements.
+     * @return A list of visible elements.
+     */
+    public static  List<Element> getVisibleElements(NodeList<Element> nodeList) {
+        List<Element> visibleElements = new ArrayList<>();
+        for (int i = 0; i < nodeList.getLength(); i++) {
+            Element element = nodeList.getItem(i);
+            if (DomUtil.isVisible(element) && DomUtil.isVisibleByItsOffset(element)) {
+                visibleElements.add(element);
+            }
+        }
+        return visibleElements;
+    }
+
     /*
      * We want to use jsni for direct access to javascript's innerText.  This avoids GWT's
      * implementation of Element::getInnerText(), which is intentionally different to mimic an old
      * IE behaviour, which returns text within <script> tags.
      */
     public static native String getInnerText(Node node) /*-{
         return node.innerText;
     }-*/;
 
     public static native double getTime() /*-{
@@ skipping 49 matching lines @@
      */
     public static Node getNearestCommonAncestor(final Node n1, final Node n2) {
         Node parent = n1;
         while (parent != null && !JavaScript.contains(parent, n2)) parent = parent.getParentNode();
         return parent;
     }
 
     /**
      * Get the nearest common ancestor of nodes.
      */
-    public static Node getNearestCommonAncestor(final NodeList ns) {
-        if (ns.getLength() == 0) return null;
-        Node parent = ns.getItem(0);
-        for (int i = 1; i < ns.getLength(); i++) {
-            parent = getNearestCommonAncestor(parent, ns.getItem(i));
+    public static Node getNearestCommonAncestor(final List<Element> ns) {
+        if (ns.size() == 0) return null;
+        Node parent = ns.get(0);
+        for (int i = 1; i < ns.size(); i++) {
+            parent = getNearestCommonAncestor(parent, ns.get(i));
         }
         return parent;
     }
 
     /**
      * Get all text from a tree/sub-tree.
      * @param node The root of the tree.
      * @return The text contained in this tree.
      */
     public static String getTextFromTree(Node node) {
@@ skipping 205 matching lines @@
     }-*/;
 
     public static native Document createHTMLDocument(Document doc) /*-{
         return doc.implementation.createHTMLDocument();
     }-*/;
 
     public static native Element getFirstElementChild(Document document) /*-{
         return document.firstElementChild;
     }-*/;
 }