Call FPDFAvail_IsFormAvail() before creating form.

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 0528c94ecca007a1e58ec053c96689974027318d..1739f7192a091f1acca5ec5fbc55a324be42c92c 100644
--- a/pdf/pdfium/pdfium_engine.cc
+++ b/pdf/pdfium/pdfium_engine.cc
@@ -2597,11 +2597,15 @@ void PDFiumEngine::ContinueLoadingDocument(
   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(