Support <picture> in image extraction

java/org/chromium/distiller/extractors/embeds/ImageExtractor.java

 // Copyright 2015 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.extractors.embeds;
 
 import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.ImageElement;
 import com.google.gwt.dom.client.NodeList;
@@ skipping 11 matching lines @@
  */
 public class ImageExtractor implements EmbedExtractor {
     private static final Set<String> relevantTags = new HashSet<>();
     private String imgSrc;
     private int width;
     private int height;
 
     static {
         // TODO(mdjones): Add "DIV" to this list for css images and possibly captions.
         relevantTags.add("IMG");
+        relevantTags.add("PICTURE");
         relevantTags.add("FIGURE");
     }
 
     private static final String[] LAZY_IMAGE_ATTRIBUTES =
             {"data-src", "data-original", "datasrc", "data-url"};
 
     @Override
     public Set<String> getRelevantTagNames() {
         return relevantTags;
     }
 
     @Override
     public WebImage extract(Element e) {
         if (!relevantTags.contains(e.getTagName())) {
             return null;
         }
         imgSrc = "";
 
         if ("IMG".equals(e.getTagName())) {
             extractImageAttributes(ImageElement.as(e));
             return new WebImage(e, width, height, imgSrc);
+        } else if ("PICTURE".equals(e.getTagName())) {

[mdjones, 2017/01/17 17:41:46]

How would you feel about having some sort of utility function in this class
called "getImageElement"? That way we could eliminate the duplicate
"img"/"picture" tag checks here and below. It's not that hard to read now, but
adding another tag (maybe div) would further complicate this. wdyt? 

[wychen, 2017/01/18 17:29:54]

Done.

+                return new WebImage(e, width, height, imgSrc);

[mdjones, 2017/01/17 17:41:47]

nit: indented too far.

[wychen, 2017/01/18 17:29:54]

Done.

         } else if ("FIGURE".equals(e.getTagName())) {
-            Element img = getFirstElementByTagName(e, "IMG");
+            Element img = getFirstElementByTagName(e, "PICTURE");
+            if (img == null) {
+                img = getFirstElementByTagName(e, "IMG");
+            }
             if (img != null) {

[mdjones, 2017/01/17 17:41:46]

nit: early return instead of nesting?

[wychen, 2017/01/18 17:29:54]

Done.

-                extractImageAttributes(ImageElement.as(img));
+                if ("IMG".equals(img.getTagName())) {
+                    extractImageAttributes(ImageElement.as(img));
+                }
                 Element figcaption;
                 Element cap = getFirstElementByTagName(e, "FIGCAPTION");
                 if (cap != null) {
                     // We look for links because some sites put non-caption
                     // elements into <figcaption>. For example: image credit
                     // could contain a link. So we get the whole DOM structure within
                     // <figcaption> only when it contains links, otherwise we get the innerText.
                     figcaption = getFirstElementByTagName(cap, "A") != null ?
                             cap : createFigcaptionElement(cap);
                 } else {
@@ skipping 38 matching lines @@
         }
         return null;
     }
 
     private Element createFigcaptionElement(Element element) {
         Element figcaption = Document.get().createElement("FIGCAPTION");
         figcaption.setInnerText(DomUtil.getInnerText(element));
         return figcaption;
     }
 }