Discard hidden articles when using fast path

javatests/org/chromium/distiller/ContentExtractorTest.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.dom.client.Document;
 import com.google.gwt.dom.client.Element;
 
 public class ContentExtractorTest extends DomDistillerJsTestCase {
@@ skipping 478 matching lines @@
         mBody.setInnerHTML("");
         Element div = TestUtil.createDiv(0);
         mBody.appendChild(div);
 
         div.setInnerHTML(html);
         ContentExtractor extractor = new ContentExtractor(mRoot);
         String extractedContent = extractor.extractContent();
         assertEquals(expected, TestUtil.removeAllDirAttributes(extractedContent));
     }
 
     public void testOnlyProcessArticleElement() {

[wychen, 2015/11/13 07:10:57]

It might be better to directly test getArticleElement() instead of indirectly
through ContentExtractor. 

Would you mind do the following tasks?
- Move getArticleElement() and the functions it depends on to DomUtil as public
static functions.
- Move all the getArticleElement() tests here to DomUtilTest.
- Instead of doing assertExtractor(), do this:

private Element getArticleElement(String html) {
    mBody.setInnerHTML(html);
    return DomUtil.getArticleElement(mRoot);
}

then

assertNull(getArticleElement(html));
  or
assertNotNull(getArticleElement(htmlArticle));

         final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
 
         final String html = "<h1>" + CONTENT_TEXT + "</h1><div>" + article + "</div>";
         final String expected = "<h1>" + CONTENT_TEXT + "</h1>" + article;
 
         // Make sure everything is there before using the fast path.
         assertExtractor(expected, html);
 
         final String htmlArticle =
             "<h1>" + CONTENT_TEXT + "</h1>" +
             "<article>" + article + "</article>";
 
         assertExtractor(article, htmlArticle);
     }
 
+    public void testOnlyProcessArticleElementWithHiddenArticleElement() {
+        final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
+
+        final String htmlArticle =
+            "<h1>" + CONTENT_TEXT + "</h1>" +
+            "<article>" + article + "</article>" +
+            "<article style=\"display:none\">" + article + "</article>";
+
+        assertExtractor(article, htmlArticle);
+    }
+
     public void testOnlyProcessArticleElementMultiple() {
         final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
 
         final String htmlArticle =
             "<h1>" + CONTENT_TEXT + "</h1>" +
             "<article>" + article + "</article>" +
             "<article>" + article + "</article>";
         final String expected = "<h1>" + CONTENT_TEXT + "</h1>" + article + article;
 
         // The existence of multiple articles disables the fast path.
         assertExtractor(expected, htmlArticle);
     }
 
+    public void testOnlyProcessArticleElementMultipleWithHiddenArticleElement() {
+        final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
+
+        final String htmlArticle =
+            "<h1>" + CONTENT_TEXT + "</h1>" +
+            "<article>" + article + "</article>" +
+            "<article style=\"display:none\">" + article + "</article>" +
+            "<article>" + article + "</article>";
+        final String expected = "<h1>" + CONTENT_TEXT + "</h1>" + article + article;
+
+        // The existence of multiple articles disables the fast path.
+        assertExtractor(expected, htmlArticle);
+    }
+
     public void testOnlyProcessOGArticle() {
         final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
 
         final String htmlArticle =
             "<h1>" + CONTENT_TEXT + "</h1>" +
             "<div itemscope itemtype=\"http://schema.org/Article\">" + article + "</div>";
 
         assertExtractor(article, htmlArticle);
     }
 
+    public void testOnlyProcessOGArticleWithHiddenArticleElement() {
+        final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
+
+        final String htmlArticle =
+            "<h1>" + CONTENT_TEXT + "</h1>" +
+            "<div itemscope itemtype=\"http://schema.org/Article\">" + article + "</div>" +
+            "<div itemscope itemtype=\"http://schema.org/Article\" style=\"display:none\">" +
+                    article + "</div>";
+
+        assertExtractor(article, htmlArticle);
+    }
+
     public void testOnlyProcessOGArticleNews() {
         final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
 
         final String htmlArticle =
             "<h1>" + CONTENT_TEXT + "</h1>" +
             "<div itemscope itemtype=\"http://schema.org/NewsArticle\">" + article + "</div>";
 
         assertExtractor(article, htmlArticle);
     }
 
+    public void testOnlyProcessOGArticleNewsWithHiddenArticleElement() {
+        final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
+
+        final String htmlArticle =
+            "<h1>" + CONTENT_TEXT + "</h1>" +
+            "<div itemscope itemtype=\"http://schema.org/NewsArticle\">" + article + "</div>" +
+            "<div itemscope itemtype=\"http://schema.org/NewsArticle\" style=\"display:none\">" +
+                    article + "</div>";
+
+        assertExtractor(article, htmlArticle);
+    }
+
     public void testOnlyProcessOGArticleBlog() {
         final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
 
         final String htmlArticle =
             "<h1>" + CONTENT_TEXT + "</h1>" +
             "<div itemscope itemtype=\"http://schema.org/BlogPosting\">" + article + "</div>";
 
         assertExtractor(article, htmlArticle);
     }
 
+    public void testOnlyProcessOGArticleBlogWithHiddenArticleElement() {
+        final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
+
+        final String htmlArticle =
+            "<h1>" + CONTENT_TEXT + "</h1>" +
+            "<div itemscope itemtype=\"http://schema.org/BlogPosting\">" + article + "</div>" +
+            "<div itemscope itemtype=\"http://schema.org/BlogPosting\" style=\"display:none\">" +
+                    article + "</div>";
+
+        assertExtractor(article, htmlArticle);
+    }
+
     public void testOnlyProcessOGArticleNested() {
         final String paragraph = "<p>" + CONTENT_TEXT + "</p>";
         final String article = paragraph + paragraph;
 
         final String htmlArticle =
             "<h1>" + CONTENT_TEXT + "</h1>" +
             "<div itemscope itemtype=\"http://schema.org/Article\">" +
                 paragraph +
                 "<div itemscope itemtype=\"http://schema.org/Article\">" + paragraph + "</div>" +
             "</div>";
 
         assertExtractor(article, htmlArticle);
     }
 
+    public void testOnlyProcessOGArticleNestedWithNestedHiddenArticleElement() {
+        final String paragraph = "<p>" + CONTENT_TEXT + "</p>";
+        final String article = paragraph + paragraph;
+
+        final String htmlArticle =
+            "<h1>" + CONTENT_TEXT + "</h1>" +
+            "<div itemscope itemtype=\"http://schema.org/Article\">" +
+                paragraph +
+                "<div itemscope itemtype=\"http://schema.org/Article\">" + paragraph + "</div>" +
+                "<div itemscope itemtype=\"http://schema.org/Article\" style=\"display:none\">" +
+                    article + "</div>" +
+            "</div>";
+
+        assertExtractor(article, htmlArticle);
+    }
+
+    public void testOnlyProcessOGArticleNestedWithHiddenArticleElement() {
+        final String paragraph = "<p>" + CONTENT_TEXT + "</p>";
+        final String article = paragraph + paragraph;
+
+        final String htmlArticle =
+             "<h1>" + CONTENT_TEXT + "</h1>" +
+             "<div itemscope itemtype=\"http://schema.org/Article\">" +
+                 paragraph +
+                 "<div itemscope itemtype=\"http://schema.org/Article\">" + paragraph + "</div>" +
+             "</div>" +
+             "<div itemscope itemtype=\"http://schema.org/Article\" style=\"display:none\">" +
+                 article + "</div>";
+
+        assertExtractor(article, htmlArticle);
+    }
+
     public void testOnlyProcessOGNonArticleMovie() {
         final String article = "<p>" + CONTENT_TEXT + "</p><p>" + CONTENT_TEXT + "</p>";
 
         final String htmlArticle =
             "<h1>" + CONTENT_TEXT + "</h1>" +
             "<div itemscope itemtype=\"http://schema.org/Movie\">" + article + "</div>";
         final String expected = "<h1>" + CONTENT_TEXT + "</h1>" + article;
 
         // Non-article schema.org types should not use the fast path.
         assertExtractor(expected, htmlArticle);
     }
 }