Age | Commit message (Collapse) | Author |
|
CloneNonCycle() tries to detect cyclic object references without copying
them. There are two issues:
-- for elements in an array or a dictionary, they should be able to
refer to the same object, which are not cyclic;
-- for cyclic referenced elements in an array or a dictionary, do not
clone the element at all. Having nullptr or <key, nullptr> as an element,
like we did before, might cause crash when the element being accessed.
BUG=chromium:701860
Change-Id: Id0304accde76ed06fa5ce640994c7628359600fb
Reviewed-on: https://pdfium-review.googlesource.com/3156
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
|
|
When a web link has a hyphen at the end of line, we consider it to
be continued to the next line. For example, "http://www.abc.com/my-\r\ntest"
should be extracted as "http://www.abc.com/my-test".
BUG=pdfium:650
Change-Id: I64a93d9c66faf2be0abdaf8cfe8ee496c435d0ca
Reviewed-on: https://pdfium-review.googlesource.com/3092
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
|
|
JS Array Buffers are the first candidate to be allocated from
PartitionAlloc when it becomes available, so add test first.
Presently, we will return as large an array buffer as the system
can handle; this is generally a bad idea so limit them to 256MB
and test that we handle failure.
Change-Id: I205745a7938d69eb32ac883b90824f2f9e584ec7
Reviewed-on: https://pdfium-review.googlesource.com/3065
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
|
|
The expected way to create native PDFium objects for JS is via
the NewFxDynamicObject() call in C++, but that doesn't mean that the
corresponding constructors won't be called from JS. In that case,
the internal fields will be uninitialized, and subsequent method
calls may try to use them.
Add a constructor callback for all PDFium objects that nulls out
these fields (shame that v8 doesn't do this by default, but probably
saves some cycles). Then ensure that we check for this possibility
in all the places it might turn up.
Conversely, if we've just gotten a successful return from
NewFxDynamicObject(), we know the CJS_Object/EmbedObj are good,
so avoid checking there.
BUG=695826
Change-Id: Iadad644c4af937def967ddc83daac1dad7544d69
Reviewed-on: https://pdfium-review.googlesource.com/2839
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
|
|
Moving to std::vector from the more forgiving CFX_ArrayTemplate
revealed the dubious page tree traversal, which depends on the
correctness of the /Count entries to properly summarize the total
descendants under a given node.
The only "correct" thing to do is to throw away these counts as parsed,
and re-compute them, perhaps in CountPages(). But I'm not willing to do
that since it may break unknown documents in the wild.
Pass out-params as pointers while we're at it.
BUG=680376
Review-Url: https://codereview.chromium.org/2636403003
|
|
A return of ID == 0 from the embedder means the timer was not
created (see public/fpdf_formfill.h), although few embedders
actually conform to this convention.
Firing a timer with ID == 0 will thus do nothing since there can't
be such a timer in the map.
BUG=679649
Review-Url: https://codereview.chromium.org/2626863003
|
|
In this case, we observe the destruction of the object, but have
unfortunately saved a pointer to it in a local variable.
BUG=679643
Review-Url: https://codereview.chromium.org/2628233002
|
|
Widgets as returned from GetWidgets() can pop out of existence
unexpectedly, so always return observed pointers. This extends
the same pattern used elsewhere in the file to all occurrences.
BUG=679642
Review-Url: https://codereview.chromium.org/2624933002
|
|
BUG=pdfium:644
Review-Url: https://codereview.chromium.org/2581873002
|
|
BUG=pdfium:568
Review-Url: https://codereview.chromium.org/2519343002
|
|
BUG=pdfium:479
Review-Url: https://codereview.chromium.org/2521843003
|
|
BUG=chromium:667012
Review-Url: https://codereview.chromium.org/2508203007
|
|
There's a path out that deletes a pointer whose ownership
was passed off earlier. This will get simpler once more
APIs take unique_ptr.
BUG=664284
Review-Url: https://codereview.chromium.org/2495003006
|
|
This is a generic API function to retrieve any viewer preference of type
name.
BUG=pdfium:414
Review-Url: https://codereview.chromium.org/2475923003
|
|
The expectation is set incorrectly to allow the test to pass.
BUG=chromium:494057
Review-Url: https://codereview.chromium.org/2430583002
|
|
On Acrobat, if "/AP" is present on a text markup definition,
the coordinates used to draw the annotation come from "/Rect
values, whereas if "/AP" is not defined, the array defined
in /QuadPoints is used to grab the annotation coordinates from.
PDFium, on the other hand, uses "/Rect" regardless of
presence or absence of "/AP".
CL fixes PDFium to work similarly to Acrobat, in this case.
TEST=testing/resources/pixel/bug_585_*.in
BUG=pdfium:585
Review-Url: https://codereview.chromium.org/2289293005
|
|
CPDF_Font::UnicodeFromCharcode returns 0 only if ToUnicode map maps the
charcode to 0. CPDF_SimpleFont::UnicodeFromCharcode and CPDF_CID_Font::
UnicodeFromCharCode return 0 only if the call to CPDF_Font returns 0.
In other cases, these methods return an empty string. So when
processing text, a 0 return from the method should not be replaced
with the charcode.
BUG=pdfium:583
Review-Url: https://codereview.chromium.org/2342073002
|
|
In [1], it was made a mistake in the way the test case
testing/resources/pixel/bug_492.pdf was generated.
This CL aims at fixing this mistake by:
1- keep making use of the new pdfium_test capability
introduced by [1],
2- add a proper .in file for the test case to generate
its respective .pdf file.
[1] https://codereview.chromium.org/2277063003/
BUG=pdfium:492
Review-Url: https://codereview.chromium.org/2286023002
|
|
Although notably, the parameters handling support is not
complete, CL intends to be the first step towards a more
complete implementation of this API.
TEST=testing/resources/javascript/bug_492_1.in
BUG=pdfium:492
Review-Url: https://codereview.chromium.org/2281273002
|
|
In [1], the lack of support of pdfium_test to some application
level hooks was felt.
More specifically, the lack of implementation of the hook FFI_GetPage,
called when 'this.getAnnot()' is executed in an Acrobar JS context,
makes it non-trivial to JS texts that manipulate PDF annotations.
[1] https://codereview.chromium.org/2265313002/
Here is the failing call stack in pdfium_test:
0 ::RenderPdf (samples/pdfium_test.cc)
1 ::FORM_DoDocumentOpenAction (fpdfsdk/fpdfformfill.cpp)
2 CPDFSDK_Document::ProcOpenAction (fpdfsdk/fsdk_mgr.cpp)
3 CPDFSDK_ActionHandler::DoAction_DocOpen (fpdfsdk/fsdk_actionhandler.cpp)
<----v8---->
4 Document::getAnnot (fpdfsdk/javascript/Document.cpp)
5 CPDFSDK_Document::GetPageView (fpdfsdk/fsdk_mgr.cpp)
6 CPDFDoc_Environment::FFI_GetPage (fpdfsdk/include/fsdk_mgr.h)
(frame 6 returns nullptr, and getAnnot call in frame 4 bails)
CL extends pdfium_test app with a FFI_GetPage hook implementation.
Basically what FFI_GetPage does is returning a FPDF_PAGE instance.
In case of pdfium_test, FPDF_PAGE instances were only created on demand
when the page was going to get rendered, and then discarded.
Since FFI_GetPage can be called by JS before pages are rendered,
CL moved the page creation code into a helper function, and cached
the FPDF_PAGE instances created in a map, so it does not recreate
them needlessly.
BUG=pdfium:492
Review-Url: https://codereview.chromium.org/2277063003
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
The pointer a unique_ptr contains should be checked instead of the
pointer of the unique_ptr itself.
BUG=chromium:631912
Review-Url: https://codereview.chromium.org/2205573004
|
|
BUG=pdfium:496
Review-Url: https://codereview.chromium.org/2005653002
|
|
files. (patchset #1 id:1 of https://codereview.chromium.org/1946693002/ )"
This reverts commit a031357eaab7c934ac03717968cf78ff556c819b.
The reason to revert it is that some malformed or maliciously crafted PDF
files may cause crashes.
BUG=610973
Review-Url: https://codereview.chromium.org/1971013002
|
|
(patchset #1 id:1 of https://codereview.chromium.org/1946693002/ )
Reason for revert:
The culprit was found and confirmed, not this one.
Original issue's description:
> Revert of Relax a couple checks to allow certain non-standard PDF files. (patchset #1 id:1 of https://codereview.chromium.org/1926823002/ )
>
> Reason for revert:
> Speculatively revert due to high volume of crashes on Chromium.
>
> Original issue's description:
> > Relax a couple checks to allow certain non-standard PDF files.
> >
> > Some non-standard PDF files misuse the size of cross reference table,
> > and reuse some object number which the old one is still in use. PDFium
> > can relax the reusing of xref objects only since it is not referred in
> > the pdf document. When the size of cross reference table is larger
> > than defined, PDFium will try to continue other than abort.
> >
> > BUG=chromium:596947
> >
> > Committed: https://pdfium.googlesource.com/pdfium/+/cd1e9ff4f432cbc29ed279e6891fb7ddc2ea3734
>
> TBR=thestig@chromium.org,dsinclair@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=chromium:596947
>
> Committed: https://pdfium.googlesource.com/pdfium/+/5fc4f31285c3a88fc157fd2d9b9cf2eb5c7cabed
TBR=thestig@chromium.org,dsinclair@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:596947
Review-Url: https://codereview.chromium.org/1947983002
|
|
(patchset #1 id:1 of https://codereview.chromium.org/1926823002/ )
Reason for revert:
Speculatively revert due to high volume of crashes on Chromium.
Original issue's description:
> Relax a couple checks to allow certain non-standard PDF files.
>
> Some non-standard PDF files misuse the size of cross reference table,
> and reuse some object number which the old one is still in use. PDFium
> can relax the reusing of xref objects only since it is not referred in
> the pdf document. When the size of cross reference table is larger
> than defined, PDFium will try to continue other than abort.
>
> BUG=chromium:596947
>
> Committed: https://pdfium.googlesource.com/pdfium/+/cd1e9ff4f432cbc29ed279e6891fb7ddc2ea3734
TBR=thestig@chromium.org,dsinclair@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=chromium:596947
Review-Url: https://codereview.chromium.org/1946693002
|
|
Some non-standard PDF files misuse the size of cross reference table,
and reuse some object number which the old one is still in use. PDFium
can relax the reusing of xref objects only since it is not referred in
the pdf document. When the size of cross reference table is larger
than defined, PDFium will try to continue other than abort.
BUG=chromium:596947
Review-Url: https://codereview.chromium.org/1926823002
|
|
Some unknown software generates cross reference table with the
claimed object numbers are all off by one. Add a simple verification
function to detect this scenario, thus have a chance to rebuild
the correct cross reference table.
To avoid unnecessary checks and potiential performance hit, we only
check for the very first non free entry.
BUG=602650
Review URL: https://codereview.chromium.org/1910063004
|
|
The font_size.pdf file should not have been committed as it will be generated
at test time from the .in file.
Review URL: https://codereview.chromium.org/1908613004
|
|
https://codereview.chromium.org/1888673003/ )
Reason for revert:
Dr.Memory code is wrong.....
Original issue's description:
> Re-Reland Combined test runner.
>
> Original Commit Message:
>
> This CL revives the old CL to combine the test runners [1] which was reverted
> due to failing font_size tests. I've deleted the font_size.pdf as it is not
> needed and for testing. Pixel tests are either .in or .pdf files, not both.
>
> Original description:
> This CL takes the three test runners (corpus, javascript, pixel) and combines
> the code into a single test_runner file. Each of the individual runners still
> exists and calls the test runner with their data directory.
>
> With this change, the pixel and javascript test will now run in parallel if
> multiple processors are available.
>
> 1-https://codereview.chromium.org/1430623006/
>
> BUG=
>
> Committed: https://pdfium.googlesource.com/pdfium/+/84b26ec6ba8193235ce0df817dec5c0f1da29841
TBR=ochang@chromium.org,zhaoqin@chromium.org,tsepez@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=
Review URL: https://codereview.chromium.org/1898353004
|
|
Original Commit Message:
This CL revives the old CL to combine the test runners [1] which was reverted
due to failing font_size tests. I've deleted the font_size.pdf as it is not
needed and for testing. Pixel tests are either .in or .pdf files, not both.
Original description:
This CL takes the three test runners (corpus, javascript, pixel) and combines
the code into a single test_runner file. Each of the individual runners still
exists and calls the test runner with their data directory.
With this change, the pixel and javascript test will now run in parallel if
multiple processors are available.
1-https://codereview.chromium.org/1430623006/
BUG=
Review URL: https://codereview.chromium.org/1888673003
|
|
Fixes a potential mismatch of |m_nComponents| between CPDF_DIBSource and
its CPDF_ColorSpace, from code attempting to recover from a failed decoder
initialisation in CPDF_DIBSource::CreateDecoder.
BUG=chromium:603518
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/1892143003
|
|
https://codereview.chromium.org/1886753002/ )
Reason for revert:
Dr. Memory still broken: https://build.chromium.org/p/client.pdfium/builders/drm_win_xfa/builds/330/steps/pixel%20tests/logs/stdio
Original issue's description:
> Reland Combined test runner.
>
> This reverts commit 7a4374583efc0c41c826122aa26c1198c8d5cc37.
>
> Original Commit Message:
>
> This CL revives the old CL to combine the test runners [1] which was reverted
> due to failing font_size tests. I've deleted the font_size.pdf as it is not
> needed and for testing. Pixel tests are either .in or .pdf files, not both.
>
> Original description:
> This CL takes the three test runners (corpus, javascript, pixel) and combines
> the code into a single test_runner file. Each of the individual runners still
> exists and calls the test runner with their data directory.
>
> With this change, the pixel and javascript test will now run in parallel if
> multiple processors are available.
>
> 1-https://codereview.chromium.org/1430623006/
>
> Committed: https://pdfium.googlesource.com/pdfium/+/9cec54ab0a5461b3075c585f8f233dbfd06c2cbd
TBR=tsepez@chromium.org,ochang@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/1890703002
|
|
This reverts commit 7a4374583efc0c41c826122aa26c1198c8d5cc37.
Original Commit Message:
This CL revives the old CL to combine the test runners [1] which was reverted
due to failing font_size tests. I've deleted the font_size.pdf as it is not
needed and for testing. Pixel tests are either .in or .pdf files, not both.
Original description:
This CL takes the three test runners (corpus, javascript, pixel) and combines
the code into a single test_runner file. Each of the individual runners still
exists and calls the test runner with their data directory.
With this change, the pixel and javascript test will now run in parallel if
multiple processors are available.
1-https://codereview.chromium.org/1430623006/
Review URL: https://codereview.chromium.org/1886753002
|
|
- Using |-skew| to get positive index, which doesn't work when skew is
INT_MIN
- Incorrect logic when determining when to use |-skew| as an index.
R=tsepez@chromium.org,weili@chromium.org
BUG=chromium:601362
Review URL: https://codereview.chromium.org/1875673004
|
|
https://codereview.chromium.org/1850273003/ )
Reason for revert:
Broke Dr. Memory
Original issue's description:
> Combined test runner.
>
> This CL revives the old CL to combine the test runners [1] which was reverted
> due to failing font_size tests. I've deleted the font_size.pdf as it is not
> needed and for testing. Pixel tests are either .in or .pdf files, not both.
>
> Original description:
> This CL takes the three test runners (corpus, javascript, pixel) and combines
> the code into a single test_runner file. Each of the individual runners still
> exists and calls the test runner with their data directory.
>
> With this change, the pixel and javascript test will now run in parallel if
> multiple processors are available.
>
> 1-https://codereview.chromium.org/1430623006/
>
> Committed: https://pdfium.googlesource.com/pdfium/+/03e8f12cd3788038225d0376a164a9993f6cb720
TBR=tsepez@chromium.org,thestig@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.chromium.org/1867643002
|
|
This CL revives the old CL to combine the test runners [1] which was reverted
due to failing font_size tests. I've deleted the font_size.pdf as it is not
needed and for testing. Pixel tests are either .in or .pdf files, not both.
Original description:
This CL takes the three test runners (corpus, javascript, pixel) and combines
the code into a single test_runner file. Each of the individual runners still
exists and calls the test runner with their data directory.
With this change, the pixel and javascript test will now run in parallel if
multiple processors are available.
1-https://codereview.chromium.org/1430623006/
Review URL: https://codereview.chromium.org/1850273003
|
|
Review URL: https://codereview.chromium.org/1838543002
|
|
R=dsinclair@chromium.org
Review URL: https://codereview.chromium.org/1837543002
|
|
Added test case.
Several bugs going on here:
JS_LocalTime() ignoring argument and returning current time
and not factoring in the time zone adjustment.
Use of FXSYS_floor() silently casts result to float,
losing precision required to extract minutes and seconds.
Pre-existing wcsftime escapes not stripped.
BUG=pdfium:413
Review URL: https://codereview.chromium.org/1833053002
|
|
Code committed at 258f19f makes the test pass.
BUG=591137
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/1779703003 .
|
|
BUG=pdfium:420
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/1757373002 .
|