PDF: Always call FPDFAvail_IsDocAvail() when loading.

pdf/pdfium/pdfium_engine.cc

Index: pdf/pdfium/pdfium_engine.cc
diff --git a/pdf/pdfium/pdfium_engine.cc b/pdf/pdfium/pdfium_engine.cc
index 5ee7e623ac43e20b3098cb3097ff2bec45443f99..c8a46a018f0be171cf2317fc6ff92f9f452ed765 100644
--- a/pdf/pdfium/pdfium_engine.cc
+++ b/pdf/pdfium/pdfium_engine.cc
@@ -298,7 +298,7 @@ FPDF_SYSFONTINFO g_font_info = {
 };
 #endif  // defined(OS_LINUX)
 
-PDFiumEngine* g_engine_for_unsupported;
+PDFiumEngine* g_engine_for_unsupported = nullptr;
 
 void Unsupported_Handler(UNSUPPORT_INFO*, int type) {
   if (!g_engine_for_unsupported) {
@@ -1049,6 +1049,7 @@ void PDFiumEngine::OnPartialDocumentLoaded() {
 
 void PDFiumEngine::OnPendingRequestComplete() {
   if (!doc_ || !form_) {
+    DCHECK(fpdf_availability_);

[Lei Zhang, 2016/05/25 17:16:36]

I'm pretty sure OnPartialDocumentLoaded() has to be called before we can get to
this function.

[Wei Li, 2016/05/25 22:37:38]

Acknowledged.

     LoadDocument();
     return;
   }
@@ -2430,11 +2431,17 @@ bool PDFiumEngine::TryLoadingDoc(const std::string& password,
   } else {
     doc_ = FPDFAvail_GetDocument(fpdf_availability_, password_cstr);
   }
+  if (!doc_) {
+    if (FPDF_GetLastError() == FPDF_ERR_PASSWORD)
+      *needs_password = true;
+    return false;
+  }
 
-  if (!doc_ && FPDF_GetLastError() == FPDF_ERR_PASSWORD)
-    *needs_password = true;
-
-  return !!doc_;
+  // Always call FPDFAvail_IsDocAvail() so PDFium initializes internal data
+  // structures.
+  int avail = FPDFAvail_IsDocAvail(fpdf_availability_, &download_hints_);

[Lei Zhang, 2016/05/25 17:16:36]

I decided to (potentially) call this a second time just after loading the
document instead, like we do in samples/pdfium_test.cc.

[Wei Li, 2016/05/25 22:37:38]

Would this approach be fragile? In samples/pdfium_test.cc, it uses a loop to
wait until it becomes available. FPDFAvail_IsDocAvail calls
CPDF_DataAvail::CheckDocStatus which requires to be called at least three times
to read the hint table. 

[Lei Zhang, 2016/06/14 17:53:56]

Whether FPDFAvail_IsDocAvail() ends up reading the hint table depends on the
document. I think the DCHECK assertion may be a bit strong and I will remove it.
The goal here is not to make sure it returns PDF_DATA_AVAIL. (though it probably
should for well-formed documents) Instead, we just want to make sure
FPDFAvail_IsDocAvail() gets called at least once, so it has a chance to progress
through the CheckDocStatus() calls.

I tested with a modified pdfium_test where I don't run it in a loop and it
works.

[Wei Li, 2016/06/14 23:33:42]

I think the fix is in right direction. But we need to be extra careful here. If
I understand correctly, if the pdf file is not available at this moment, the
subsequent LoadDocument/FPDFAvail_IsDocAvail will not be called any more since
doc_ is set already. 

+  DCHECK_EQ(PDF_DATA_AVAIL, avail);
+  return true;
 }
 
 void PDFiumEngine::GetPasswordAndLoad() {