Report empty article as empty.

ios/chrome/browser/dom_distiller/distiller_viewer.cc

 // 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.
 
 #include "ios/chrome/browser/dom_distiller/distiller_viewer.h"
 
 #include <string>
 #include <utility>
 
 #include "components/dom_distiller/core/distilled_page_prefs.h"
@@ skipping 20 matching lines @@
   std::unique_ptr<ViewerHandle> viewer_handle = distillerService->ViewUrl(
       this, distillerService->CreateDefaultDistillerPage(gfx::Size()), url);
 
   TakeViewerHandle(std::move(viewer_handle));
 }
 
 DistillerViewer::~DistillerViewer() {}
 
 void DistillerViewer::OnArticleReady(
     const dom_distiller::DistilledArticleProto* article_proto) {
   DomDistillerRequestViewBase::OnArticleReady(article_proto);

[wychen, 2016/12/22 07:58:19]

In DomDistillerRequestViewBase::OnArticleReady(), "No data found" is added to
js_buffer_.

[Olivier, 2016/12/22 10:54:59]

But IIUC, not the protobuf, correct?
Should I do my check before this call?

[wychen, 2016/12/22 11:05:12]

The protobuf should be intact, so no need to change. I was thinking about
calling it conditionally, but it does other things.

-  if (article_proto->pages_size() > 0) {
+  bool is_empty = true;
+  for (int i = 0; i < article_proto->pages_size(); i++) {

[wychen, 2016/12/22 10:19:33]

When the first distilled page is empty, even if the next page link is
discovered, the link would be disregarded. The logic is here:

https://cs.chromium.org/chromium/src/components/dom_distiller/core/distiller....

We did this because the distillability heuristic could still be wrong, and if
the page is not an article, stitching a paginated part would be moot.

Given this, we only need to check the first page here.

Your current implementation is defensive, and I'm fine with it.

[Olivier, 2016/12/22 10:54:59]

Done. This makes code simpler

+    if (article_proto->pages(i).html() != "") {

[wychen, 2016/12/22 10:19:33]

Using .empty() might be slightly better.

[Olivier, 2016/12/22 10:54:59]

Done.

+      is_empty = false;
+      break;
+    }
+  }
+  if (article_proto->pages_size() > 0 && !is_empty) {
     std::vector<ImageInfo> images;
     for (int i = 0; i < article_proto->pages(0).image_size(); i++) {
       auto image = article_proto->pages(0).image(i);
       images.push_back(ImageInfo{GURL(image.url()), image.data()});
     }
-
     const std::string html = viewer::GetUnsafeArticleTemplateHtml(
         url_.spec(), distilled_page_prefs_->GetTheme(),
         distilled_page_prefs_->GetFontFamily());
 
     std::string html_and_script(html);
     html_and_script +=
         "<script> distiller_on_ios = true; " + js_buffer_ + "</script>";
     callback_.Run(url_, html_and_script, images, article_proto->title());
   } else {
     callback_.Run(url_, std::string(), {}, std::string());
   }
 }
 
 void DistillerViewer::SendJavaScript(const std::string& buffer) {
   js_buffer_ += buffer;
 }
 
 }  // namespace dom_distiller