Age | Commit message (Collapse) | Author |
|
Review-Url: https://codereview.chromium.org/2276953003
|
|
Currently the only calls to CloseParser() happend in the destructor or the
start*Parse methods. The Start*Parse methods are currently only called on
freshly constructed parsers in fpdf_dataavail and fpdfview.
This CL removes the CloseParser() method and puts the contents in the
destructor. We then add an ASSERT that we don't re-enter the parser after it
has already completed the parse.
Review-Url: https://codereview.chromium.org/2267173005
|
|
This CL makes some methods private which are only used internally, removes
unused methods and removes an unused class.
Review-Url: https://codereview.chromium.org/2278583002
|
|
This Cl switches the ownership between the parser and the document. Previously
the parser owned the document and we'd jump through hoops during cleanup to
delete the right object. This Cl flips the ownership so the document owns
the parser and simplifies the cleanup logic where needed.
BUG=pdfium:565
Review-Url: https://codereview.chromium.org/2275773003
|
|
For some complex objects such as CPDF_Dictionary, CPDF_Array,
CPDF_Stream, and CPDF_Reference, Clone() could be executed with
infinite recursion to cause the stack overflow. Fix this by
checking already cloned objects to avoid recursion.
BUG=pdfium:513
Review-Url: https://codereview.chromium.org/2250533002
|
|
Embedder test's delegate function GetPage() calls FPDF_LoadPage() to
load a page which may be already loaded by embedder test itself.
Thus the page's ref count is increased unnecessarily.
This causes the page to be leaked. Fix this by putting the page map in
embedder test class and guarantee the page is loaded only once.
Also, fix leaks in this embedder tests by unloading the loaded pages to
properly release the resource.
BUG=pdfium:242
Review-Url: https://codereview.chromium.org/2258333002
|
|
Following up on [1], where the duplicated logic in
Field::SetDisplay was factored out into a helper function,
CL further cleans up a related method: ::SetHidden.
Field::SetHidden(true), for instance, is equivalent
to calling Field::SetDisplay(1), whereas Field::SetHidden(false)
is equivalent to Field::SetDisplay(0);
No behavior change is expected.
[1] https://codereview.chromium.org/2255843002
Review-Url: https://codereview.chromium.org/2266193002
|
|
Currently, when we destroy a CFFL_ComboBox we'll cleanup the fontmap and then
call the destructor for the parent type. This will case the PWL_Wnd to be
destroyed. In this case, the window is a PWL_Edit. On destruction it will reset
the focus which causes the text selection to change, which asks the font map
for data but we've already destroyed the font map.
This CL forces the destruction of the window earlier in order to have the
fontmap available. A followup bug is filed to correct the location of the
fontmap so we don't have this dependency.
BUG=chromium:637546
Review-Url: https://codereview.chromium.org/2266943002
|
|
The public API FPDFPage_New() incorrectly said to use FPDFPage_Delete()
instead of FPDF_ClosePage() to free the new page. This led to a page object
leak in an embedder test. Correct the public API description
as well as its usage in the embedder test.
BUG=pdfium:242
Review-Url: https://codereview.chromium.org/2260683003
|
|
Review-Url: https://codereview.chromium.org/2262473002
|
|
The PDF specification [1] says:
"
syncAnnotScan guarantees that all annotations will be scanned
by the time this method returns.
(..)
Normally a background task runs that examine every page and
looks for annotations during idle times.
"
The statement details specifically how Acrobat implements
this method.
Although, neither the method itself nor the background scanner
task are implemented in PDFium (as of today, Ago/2016),
not having ::syncAnnotScan at least stubbed out can be considered
harmfull since its absence makes JS acrobat scripts silently
fail when it has a call to it.
Given that, and following a stub-out pattern present in other
methods including ::addAnnot and ::addField, CL provides
a stubbed out implementation of Document::syncAnnotScan.
[1] http://www.adobe.com/content/dam/Adobe/en/devnet/acrobat/pdfs/js_api_reference.pdf
BUG=pdfium:492
Review-Url: https://codereview.chromium.org/2265553002
|
|
CL implements the first step in order to support
Annotations manipulation in PDFium: Document::getAnnot.
The method takes two arguments, an integer (page number)
and a string (annotation name).
When called, it iterates over the annotations on
the given page number, searching for the one whose name
matches the string in the second parameter.
If found, then an Annot instance (see Annot.cpp/g added by this
CL), is bound to a Javascript object and returned.
With the use cases described in bug [1] as an initial test case,
CL adds support to the following Annotation object properties:
- hidden
- name
- type
Idea is to keep evolving the implementation with more methods
and properties in follow up CLs.
[1] https://bugs.chromium.org/p/pdfium/issues/detail?id=492
BUG=pdfium:492
Review-Url: https://codereview.chromium.org/2260663002
|
|
Currently the timers is a vector. When we cancel a timer we have an O(n) operation
to remove the timer. If there are a lot of timers (which this test has > 16k) this can
take a long time. The impact is a lot lower in Release, but the test is very slow in Debug.
From Linux on waterfall:
[ RUN ] FPDFFormFillEmbeddertest.BUG_634716
[ OK ] FPDFFormFillEmbeddertest.BUG_634716 (7855 ms)
From Linux try bot:
[ RUN ] FPDFFormFillEmbeddertest.BUG_634716
[ OK ] FPDFFormFillEmbeddertest.BUG_634716 (431 ms)
From Linux XFA Rel GYP on waterfall:
[ RUN ] FPDFFormFillEmbeddertest.BUG_634716
[ OK ] FPDFFormFillEmbeddertest.BUG_634716 (185 ms)
From Linux XFA Rel GYP try bot:
[ RUN ] FPDFFormFillEmbeddertest.BUG_634716
[ OK ] FPDFFormFillEmbeddertest.BUG_634716 (72 ms)
Review-Url: https://codereview.chromium.org/2251333002
|
|
If the control is not found in the CPDF_InterForm control map then
::GetControlByDict can return nullptr. This nullptr was not handled in
CPDFSDK_Widget::GetFormField(). This Cl propagates the nullptr back up to the
caller methods and fixes the returns as needed.
This is a speculative crash to fix the referenced bug.
BUG=chromium:637953
Review-Url: https://codereview.chromium.org/2256783003
|
|
Also, remove the #ifdef PDF_ENABLE_XFA that wraps the whole file.
Review-Url: https://codereview.chromium.org/2253193002
|
|
This CL moves classes in fsdk_baseform.h to their own files.
Classes include CPDFSDK_Widget, CBA_AnnotIterator, CPDFSDK_XFAWidget,
PDFSDK_FieldAction, and CPDFSDK_Interform.
Review-Url: https://codereview.chromium.org/2252723002
|
|
CL introduces a helper function to share the common
logic.
No new tests, since there is no behavior change.
Review-Url: https://codereview.chromium.org/2255843002
|
|
BUG=635848
Review-Url: https://codereview.chromium.org/2242213004
|
|
The parser and document refer to async loading and parsing. The code isn't
actually async but loading a linearized PDF. This Cl renames the methods to
clarify what the code is doing.
The LoadDoc() and LoadLinearizedDoc() methods have been refactored to share
a common LoadDocInternal() method.
Review-Url: https://codereview.chromium.org/2250163002
|
|
Precursor to someday using possibly subclassed documents.
Review-Url: https://codereview.chromium.org/2248123002
|
|
Nearly all the "loose" functions in FXJS become methods on
the CFJXS_Engine.
This is the "missing link" wrt some layering violatons that
have been around forever. We can stop passing &m_ variables
from CJS_ down into FXJS Initialization as a result.
Review-Url: https://codereview.chromium.org/2245863002
|
|
Currently the GetWidget call will create a backing widget for a control behind
the scenes if it doesn't exist. This can cause more work to be done then needed
as we may need to create the page and any supporting structures. This created
structure will then be torn down as we don't need it anymore once we're done
with the widget.
For the instances where setting the value on the widget will have no effect (as
we'll destroy it anyway) we can tell GetWidget() to not create the widget and
return without doing any work.
BUG=pdfium:632709
Review-Url: https://codereview.chromium.org/2251453002
|
|
Create a new class to hold these, CFXJS_Engine (could have been
called Runtime, but there are too many "Runtimes" already). In a
subsequent patch, all the FXJS_*() functions that take an isolate
as the first argument can become methods on the engine.
CJS_ must still manage the isolates; this happens outside
the engine.
The IJS_Runtime abstraction moves up to fpdfsdk/javascript; it
remains to allow for either a real JS library or a stubb one to
be linked (for non-js builds).
Review-Url: https://codereview.chromium.org/2241483004
|
|
This CL moves classes in fsdk_annothandler.h, which are
CPDFSDK_AnnotIterator, CPDFSDK_AnnotHandlerMgr, IPDFSDK_AnnotHandler,
CPDFSDK_XFAAnnotHandler and CPDFSDK_BFAnnotHandler, to their own files.
Review-Url: https://codereview.chromium.org/2243623002
|
|
Analogous to getting the length of JS array, this result
should be a C++-side object only.
Also rename to FXJS_GetObjectProperty to match JS nomenclature.
Review-Url: https://codereview.chromium.org/2242593002
|
|
Review-Url: https://codereview.chromium.org/2236743002
|
|
Use smart pointers instead of raw pointer to make memory management
easier for classes mainly under xfa/fxfa.
Also change the return type of IFGAS_FontMgr::Create() to smart
pointer type.
BUG=pdfium:518
Review-Url: https://codereview.chromium.org/2227883002
|
|
As per the PDF specification in [1], page 103, the
'info' property of the Document object is readonly.
[1] http://partners.adobe.com/public/developer/en/acrobat/sdk/5186AcroJS.pdf
Review-Url: https://codereview.chromium.org/2235883003
|
|
... rather then doing c-style type cast.
CL also tweaks the method body a bit to have more early-returns
rather than too-nested-if blocks.
No behavior change is expected, so no new tests are being added.
Review-Url: https://codereview.chromium.org/2230983003
|
|
files.
This is the third CL to separate fx_ge into classes, one per file.
All fx_ge.h includes had to be replaced with new includes
The method definitions for CFX_FxgeDevice were not moved to a single file.
These methods are defined in two folders different from fxge/ge, so they were left untouched for now.
Review-Url: https://codereview.chromium.org/2223213002
|
|
Review-Url: https://codereview.chromium.org/2227673005
|
|
BUG=pdfium:545
Review-Url: https://codereview.chromium.org/2229103003
|
|
CPDFSDK_ActionHandler are unclear. It does not add encapsulation, nor
simplicity to the logic.
CL is a driven-by clean up that merges CPDFSDK_FormActionHandler into
CPDFSDK_ActionHandler.
It takes the opportunity to delete CPDFSDK_ActionHandler (now) empty
constructor and destructor, in favor of compiler-generated ones.
No behavior change is expected, so no new tests are being added.
Review-Url: https://codereview.chromium.org/2227173002
|
|
BUG=pdfium:545
Review-Url: https://codereview.chromium.org/2194393002
|
|
Review-Url: https://codereview.chromium.org/2226003003
|
|
Review-Url: https://codereview.chromium.org/2224113003
|
|
Patch implements the Document's API gotoNamedDest, which is
part of the PDF specification [1], page 129, with the following
(short) description:
"Use this method to go to a named destination within the
PDF document".
[1] http://partners.adobe.com/public/developer/en/acrobat/sdk/5186AcroJS.pdf
"Named destination" is a common concept in the PDF world.
It can be used together with PDF's Links, Annotations, Bookmarks
and OpenActions, as well as an action per se, in case "this.gotoNamedDest"
is called directly.
Note that the implementation makes use of the existing hook
CPDFDoc_Environment::FFI_DoGoToAction, which ends up calling
out the embedder to actually handle it.
In case of Chromium, for instance, it calls PDFiumEngine::Form_DoGoToAction
which only handles for now the "page" property of the "destination".
Other properties, including zoom level, and scroll position
are ignored for the moment.
BUG=pdfium:492
Review-Url: https://codereview.chromium.org/2221823003
|
|
They're only related to the |app| object, not all embed's.
Alert() itself unused.
Review-Url: https://codereview.chromium.org/2224073002
|
|
The CJS_Timer is fairly far removed from JS itself, the
wrapper objects are CJS_TimerObj and TimerObj. Make it
sound less like them.
Having moved the code to app.cpp, the GlobalTimer can now
refer directly to the app, rather than its superclass of
CJS_EmbedObj. Hence we can pull some app-only timer
related methods out of the superclass.
Review-Url: https://codereview.chromium.org/2222043002
|
|
As per the PDF specification at [1]
"
This property specifies the document's URL.
".
IE/Acrobat supports it, and getting it implemented
would be one step forward in order to support Acrobat JS
script as the one in [2].
[1] http://partners.adobe.com/public/developer/en/acrobat/sdk/5186AcroJS.pdf
[2] https://bugs.chromium.org/p/pdfium/issues/detail?id=492
BUG=492
Review-Url: https://codereview.chromium.org/2219183002
|
|
This is always available elsewhere, so save the bytes and ease
removal of CJS_Array since it now only contains a single v8 local.
Review-Url: https://codereview.chromium.org/2217253002
|
|
This is the second CL in an attempt to split up the classes in fxge/include/fx_ge.h into their own files. CFX_ClipRgn is moved to core/fxge/ge because it is only used in core/fxge. The header for CFX_PathData is left in core/fxge/include since it is used elsewhere.
Review-Url: https://codereview.chromium.org/2216853004
|
|
And use it where appropriate.
Review-Url: https://codereview.chromium.org/2224623002
|
|
Fix memory ownership model for PDFium timers.
The |app| class owns the CJS_Timer as part of its vector<unique_ptr>
to them.
The CJS_Timer "owns" its slot in the global ID to timer map, and
removes itself when it is destroyed. Nothing else deletes
from the global map. Deleting from the global map is
accompanied by a callback to the embedder to clear its
resources.
Next, the proper way to remove a CJS_Timer is by going
through the app, and having the app erase its unique ptr,
which then deletes the CJS_Timer, which in turn cleans up the
global map. Provide a CJS_Timer::Cancel static method to
do this conveniently.
There is a alternate path to the CJS_timer via JS and its
CJS_TimerObj. CJS_TimerObj owns a TimerObj that currently
points to the CJS_Timer. If the timer fires, and cleans
itself up, this can go stale.
Make the TimerObj maintain a weak reference via global
timer ID rather than a direct pointer to the CJS_Timer, so
that if the timer fires and is destroyed, future attempts
to cancel find nothing.
There is another path, where if the JS timer object is GC'd, then we
just clean up its CJS_TimerObj without touching
the actual CJS_Timers. We could make this match the spec
by calling into the new cancel routine as described above,
but it seems weird to have a timer depend on whether a gc
happened or not.
A subsequent CL will rename these objects to more closely
match the conventions used by the other JS wrappers.
BUG=634716
Review-Url: https://codereview.chromium.org/2221513002
|
|
Also get rid of FXJS_ValueCopy() while we're at it.
BUG=pdfium:556
Review-Url: https://codereview.chromium.org/2215093002
|
|
This is the first CL in an attempt to split up the classes in fxge/include/fx_ge.h into their own files.
Review-Url: https://codereview.chromium.org/2217663002
|
|
We must look the timer up a second time since the callback
may have released it.
BUG=634394
Review-Url: https://codereview.chromium.org/2214003003
|
|
Adds more questionable invocations of ClearTimeOut().
Also, checking that nothing happened is fragile. Log at least
one thing to show that the code ran.
Review-Url: https://codereview.chromium.org/2218473002
|
|
Review-Url: https://codereview.chromium.org/2206283004
|
|
While we're at it, beef up existing test for non-cancellation.
In turn, fix test harness to implement intervals properly.
In turn, fix public documentation to be clearer about timers.
Also rename a few identifiers that sounded "off".
Review-Url: https://codereview.chromium.org/2211513002
|