Call FPDFAvail_IsFormAvail() before creating form.

pdf/pdfium/pdfium_engine.cc

 // Copyright (c) 2012 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 "pdf/pdfium/pdfium_engine.h"
 
 #include <math.h>
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 2579 matching lines @@
     return;
   }
 
   if (FPDFDoc_GetPageMode(doc_) == PAGEMODE_USEOUTLINES)
     client_->DocumentHasUnsupportedFeature("Bookmarks");
 
   permissions_ = FPDF_GetDocPermissions(doc_);
   permissions_handler_revision_ = FPDF_GetSecurityHandlerRevision(doc_);
 
   if (!form_) {
-    // Only returns 0 when data isn't available.  If form data is downloaded, or
-    // if this isn't a form, returns positive values.
-    if (!doc_loader_.IsDocumentComplete() &&
-        !FPDFAvail_IsFormAvail(fpdf_availability_, &download_hints_)) {
-      return;
+    if (fpdf_availability_) {

[spelchat, 2015/12/30 19:01:08]

This check and DCHECK_NE could be removed instead. This is more in-line with
what's done in the rest of the file, but it seems odd to me to call
PDFAvail_IsFormAvail() with invalid arguments and just ignore the error.

[Lei Zhang, 2015/12/30 19:58:19]

I think for this to happen, the error from FPDFAvail_IsDocAvail() in
LoadDocument() got ignored as well.

Looking at the possible call stacks, OnPartialDocumentLoaded() is the only place
that calls FPDFAvail_Create() and FPDFAvail_Create() can't fail. However,
sometimes LoadDocument() is called from OnPendingRequestComplete() or
OnDocumentComplete() instead. It's not obvious if OnPartialDocumentLoaded()
should have been called already, but clearly there are cases where it did not
get called, thus needing this check.

Should we just make sure FPDFAvail_Create() gets called instead?

[spelchat, 2015/12/30 20:14:00]

Yes all the FPDFAvail_* functions get called but return an error which is
ignored. That's what I meant when I mentioned it would be more in-line with the
rest of the file to just remove this check and DCHECK_NE. Thinking about it now
I do think that's preferable to what I have now (although I'm not a big fan of
calling those functions when it doesn't make sense...).

I think all those functions only make sense if we have partial data for the
document, which is why FPDFAvail_Create() is only called in
OnPartialDocumentLoaded(). If the document has been fully downloaded before
LoadDocument() is called, then FPDFAvail_Create() is not called and all the
FPDFAvail functions return early.

I don't know if there would be a problem in using the same path for both cases
(loading with partial data or loading once the whole document is loaded). I
suspect the FPDFAvail_* functions may fail on non-linearized PDFs.

+      int form_status =
+          FPDFAvail_IsFormAvail(fpdf_availability_, &download_hints_);
+      DCHECK_NE(form_status, PDF_FORM_ERROR);
+      bool doc_complete = doc_loader_.IsDocumentComplete();
+      // Try again if the data is not available and the document hasn't finished
+      // downloading.
+      if (form_status == PDF_FORM_NOTAVAIL && !doc_complete)
+        return;
     }
 
     form_ = FPDFDOC_InitFormFillEnvironment(
         doc_, static_cast<FPDF_FORMFILLINFO*>(this));
 #ifdef PDF_USE_XFA
     FPDF_LoadXFA(doc_);
 #endif
 
     FPDF_SetFormFieldHighlightColor(form_, 0, kFormHighlightColor);
     FPDF_SetFormFieldHighlightAlpha(form_, kFormHighlightAlpha);
@@ skipping 1384 matching lines @@
     double* height) {
   FPDF_DOCUMENT doc = FPDF_LoadMemDocument(pdf_buffer, pdf_buffer_size, NULL);
   if (!doc)
     return false;
   bool success = FPDF_GetPageSizeByIndex(doc, page_number, width, height) != 0;
   FPDF_CloseDocument(doc);
   return success;
 }
 
 }  // namespace chrome_pdf