New issue
Advanced search Search tips

Issue 766790 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

File picker button in testing/corpus/third_party/tcpdf/example_054.pdf crashes

Project Member Reported by hnakashima@chromium.org, Sep 19 2017

Issue description

1. Open pdfium/testing/corpus/third_party/tcpdf/example_054.pdf
2. Click the "*" button on the "file:" line.

Expected: file picker opens, or nothing happens if feature not supported.
Actual: in debug, crash. In release, nothing.

[1:1:0919/153959.313165:FATAL:out_of_process_instance.cc(1374)] Check failed: false. 
#0 0x7fb008199e9d base::debug::StackTrace::StackTrace()
#1 0x7fb00819826c base::debug::StackTrace::StackTrace()
#2 0x7fb00822904a logging::LogMessage::~LogMessage()
#3 0x556e4af653f2 chrome_pdf::OutOfProcessInstance::ShowFileSelectionDialog()
#4 0x556e4af65500 chrome_pdf::OutOfProcessInstance::ShowFileSelectionDialog()
#5 0x556e4af1717f chrome_pdf::PDFiumEngine::Form_Browse()
#6 0x556e4afbf2da CPDFSDK_FormFillEnvironment::JS_fieldBrowse()
#7 0x556e4b3ada6e Field::browseForFileToSubmit()
#8 0x556e4b3c7802 JSMethod<>()
#9 0x556e4b3b6366 CJS_Field::browseForFileToSubmit_static()
#10 0x7faff8af2b82 v8::internal::FunctionCallbackArguments::Call()
#11 0x7faff8bee633 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#12 0x7faff8bec823 v8::internal::Builtin_Impl_HandleApiCall()
#13 0x7faff8bec1ed v8::internal::Builtin_HandleApiCall()
#14 0x1c692d405204 <unknown>


 
Description: Show this description
Cc: tsepez@chromium.org
Should we let PDFs do this? If not, just remove the NOTREACHED() since it's being reached.

Comment 3 by tsepez@chromium.org, Sep 19 2017

I think we want to let PDFium continue to do this from JS for the sake of other embedders, so chrome should silently return an error code without crashing.

Comment 4 by tsepez@chromium.org, Sep 19 2017

https://cs.chromium.org/chromium/src/third_party/pdfium/fpdfsdk/cpdfsdk_formfillenvironment.cpp?rcl=5b2092a1ec59077b430bd2cab91554cad2eb5128&l=113 calls this a coupla times, which is weird -- seems like the 'query length first time' API doesn't mate well with user experience -- select it twice?

Comment 5 by tsepez@chromium.org, Sep 19 2017

Maybe pdfium_engine just leaves this callback as null. 

IPDF_JSPLATFORM::Field_browse = Form_Browse;
Owner: rharrison@chromium.org
Status: Started (was: Untriaged)
Removing the callback appears to work
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/675bc8613fee3cbdd85ce2adb5d5cb842c9c8a49

commit 675bc8613fee3cbdd85ce2adb5d5cb842c9c8a49
Author: Ryan Harrison <rharrison@chromium.org>
Date: Fri Oct 13 20:32:24 2017

Change Field_browse callback to nullptr

This removes the support for this feature in Chrome's PDF Viewer,
which is desired since it eventually comes back to a NOTREACHED(). But
also allows PDFium to continue to support the call back so that other
embedders can implement if they so desire.

BUG= chromium:766790 

Change-Id: I76a9f75f32779f0c0d966321c647d6d42ec51c1e
Reviewed-on: https://chromium-review.googlesource.com/713735
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508803}
[modify] https://crrev.com/675bc8613fee3cbdd85ce2adb5d5cb842c9c8a49/pdf/pdfium/pdfium_engine.cc
[modify] https://crrev.com/675bc8613fee3cbdd85ce2adb5d5cb842c9c8a49/pdf/pdfium/pdfium_engine.h

Status: Fixed (was: Started)

Sign in to add a comment