Age | Commit message (Collapse) | Author |
|
id:120001 of https://codereview.chromium.org/2375343004/ )
Reason for revert:
Broke PDFExtensionTest when rolling DEPS in Chromium.
Original issue's description:
> Assert that only 0-numbered objects are Released()
>
> This condition holds because numbered objects are brute-force
> deleted by the indirect object holder, rather than being
> released.
>
> Be careful about recursive deletion, check before advancing,
> since we no longer count on Release() doing this for us.
> Fix a few tests where the test was violating ownership rules.
>
> This should be the last step before completely removing Release()
> in favor of direct delete everywhere.
>
> Committed: https://pdfium.googlesource.com/pdfium/+/aba528a362248a54b27a7e9e046e2b65ab83f624
TBR=tsepez@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/2387193003
|
|
This condition holds because numbered objects are brute-force
deleted by the indirect object holder, rather than being
released.
Be careful about recursive deletion, check before advancing,
since we no longer count on Release() doing this for us.
Fix a few tests where the test was violating ownership rules.
This should be the last step before completely removing Release()
in favor of direct delete everywhere.
Review-Url: https://codereview.chromium.org/2375343004
|
|
Review-Url: https://codereview.chromium.org/2386273004
|
|
|Clear| is too easily mistaken for "clear this pointer only."
Review-Url: https://codereview.chromium.org/2385303002
|
|
The original way of detecting loops was passing a level parameter
through various functions. This missed some cases which also lead
to load type3 font char, for example, FindFont() may call
CheckType3FontMetrics() which may eventually lead to LoadChar().
The new way is to store the char loading depth, and abort when the depth
exceeds the max.
BUG=chromium:651304
Review-Url: https://codereview.chromium.org/2384853002
|
|
Review-Url: https://codereview.chromium.org/2386433002
|
|
Restore CPDF_Dictionary default constructor.
Use it in places where reasonable in the code.
TBR=dsinclair@chromium.org
TBR=thestig@chromium.org
Review-Url: https://codereview.chromium.org/2383843002
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2377393002
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2382723003
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2381063002
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2374383003
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2376153004
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2382763002
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2383543002
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2379033002
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2375283003
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2381863003
|
|
BUG=pdfium:611
Review-Url: https://codereview.chromium.org/2374413002
|
|
It is not necessary. For some methods, bool can be used instead. For
some other methods, the returned boolean is never used, so they can be
void.
Review-Url: https://codereview.chromium.org/2382803002
|
|
All usage were refactored in commit 9972ff99.
Review-Url: https://codereview.chromium.org/2377203002
|
|
And fix a typo.
TBR=tsepez@chromium.org
Review-Url: https://codereview.chromium.org/2382443004
|
|
BUG=pdfium:597
Review-Url: https://codereview.chromium.org/2345063002
|
|
This reverts commit fe0179ded8202939ea4f2b92a879b8dede7821ea.
This is blocking incremental revision, will try again.
Review-Url: https://codereview.chromium.org/2377033003
|
|
File was left behind when CPDF_GeneralStateData became
CPDF_GeneralState::StateData internal class.
Review-Url: https://codereview.chromium.org/2377843002
|
|
Review-Url: https://codereview.chromium.org/2357173005
|
|
found by libfuzzer
Review-Url: https://codereview.chromium.org/2366143002
|
|
In all cases, bool can be used instead without problems.
Review-Url: https://codereview.chromium.org/2368693002
|
|
Remove C-casts, nits.
Review-Url: https://codereview.chromium.org/2367743003
|
|
The change at 5b7c9bb differed from the original code in
that a pre-existing object would now be freed, which showed
that a collision could be possible if m_LastObjNum overflowed.
BUG=649206
Review-Url: https://codereview.chromium.org/2361303002
|
|
BUG=648935,649436
Review-Url: https://codereview.chromium.org/2360283004
|
|
Review-Url: https://codereview.chromium.org/2364643003
|
|
Specialize default_delete to avoid having to say
ReleaseDeleter<CPDF_ColorSpace> throughout the code.
Review-Url: https://codereview.chromium.org/2368433003
|
|
This better describes its purpose, and reduces confusion
with the CFX_CountRef class, which is unrelated.
The WeakPtr class itself that manipulates handles is NYI.
Review-Url: https://codereview.chromium.org/2366673003
|
|
This gives additional protection in case of re-entry. Also
make CFX_CountRef more robust in face of errors.
BUG=649229
Review-Url: https://codereview.chromium.org/2364673002
|
|
Also, it's idempotent, so simplify some logic in callers to
not care if objnum is zero.
The alternate forms are rarely used, using the objnum form
makes it clear that SetReferenceFor() can't possibly register
the object as a side-effect.
Review-Url: https://codereview.chromium.org/2361713002
|
|
Review-Url: https://codereview.chromium.org/2358243002
|
|
The old SetParam() method had "maybe take ownership" semanitcs
based upon the type argument.
Make GetParam() handle the None case and simplify callers
based upon that behaviour.
Review-Url: https://codereview.chromium.org/2358043003
|
|
Given a large enough value for the character code it's possible to overflow
the conversion to an int. This Cl updates the code to guard against overflow.
BUG=chromium:648739
Review-Url: https://codereview.chromium.org/2358023002
|
|
Review-Url: https://codereview.chromium.org/2353383002
|
|
Review-Url: https://codereview.chromium.org/2356603003
|
|
and in CPDF_CMapParser
Review-Url: https://codereview.chromium.org/2353963003
|
|
We remove the indirect object holder argument and check that
call sites pass ownable objects, adding a reference in one
place that always was passing an indirect object.
Also check that the invariant isn't violated, we need to fail
here in the wild and investigate -- these are existing UAFs.
Review-Url: https://codereview.chromium.org/2355083002
|
|
Remove CPDF_Creator and CPDF_Parser as friends of CPDF_Document. Move all
member variables to the private section, and add CPDF_TestDocument as a
private friend.
Review-Url: https://codereview.chromium.org/2349353003
|
|
Remove friendship as there doesn't appear to be anything protected that is
being accessed by CPDF_OCContext.
Review-Url: https://codereview.chromium.org/2355823002
|
|
Remove the friendship between these two classes and replace with accessor
methods.
Review-Url: https://codereview.chromium.org/2355813002
|
|
This CL renames and cleans up some methods that are similar between
CPDF_Document and CPDFXFA_Document.
Review-Url: https://codereview.chromium.org/2351673004
|
|
This reverts commit 81e1e3fd2d33478733e47bd007b76fac1a663e74.
Review-Url: https://codereview.chromium.org/2353013003
|
|
BUG=79367, 48791
The fonts was not cleared after unloading pages.
Test pdf: http://www.nasa.gov/pdf/750614main_NASA_FY_2014_Budget_Estimates-508.pdf
For this file, we have ~5 fonts per page, which equal ~1 Mb per page.
In this PDF we have 670 pages, as result after slow scrolling(reading) full document we have ~600 Mb fonts data in memory.
Memory usage of PDF Plugin:
before this CL: ~660 Mb
after this CL: ~100 Mb
This is last part of reverted CL:
Original CL: https://codereview.chromium.org/2158023002
Revert reason: BUG=647612
Fix bug CL: https://codereview.chromium.org/2350193003
Previous CL: https://codereview.chromium.org/2350243002
Review-Url: https://codereview.chromium.org/2351193002
|
|
Upon indirect object holder destruction, all indirect
objects are destroyed -- currently by order of increasing
object number -- but ideally without ordering constraints.
So currently, we can get away with a dictionary pointing
directly at an indirect object with a higher number. It
gets destroyed first, invoking Release() on its subordinates,
which skips destroying them if they are indirect objects. But
we don't want to rely on this artifact of destruction
order. Should it happen to be reversed, the dictionary
would invoke Release() on freed memory.
Interestingly, CPDF_Array skirts the issue by replacing
any indirect objects it is given with references. Not
clear whether we should do the same thing for dictionaries,
or remove it from arrays. The technique certainly
complicates understanding ownership.
The one violation found is in the unittest that broke the
previous CL which tried to use unique_ptrs in indirect
object holder.
Review-Url: https://codereview.chromium.org/2353093002
|
|
After this CL: only one global CFX_FontCache used. Any cached items
from it, are released, when they are not being used.
This is restore part of reverted CL:
Original CL: https://codereview.chromium.org/2158023002
Revert reason: BUG=647612
Fix bug CL: https://codereview.chromium.org/2350193003
Review-Url: https://codereview.chromium.org/2350243002
|