Age | Commit message (Collapse) | Author |
|
Review-Url: https://codereview.chromium.org/2353383002
|
|
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
|
|
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
|
|
This reverts commit c8544d634a1993e2592e41458be215fcd0956031.
TBR=dsinclair@chromium.org
Review URL: https://codereview.chromium.org/2355683002 .
|
|
The objects it is given are owned by it and are simply
deleted without regard to Release() used by others.
Review-Url: https://codereview.chromium.org/2350263002
|
|
We can delete this just fine on our own.
Review-Url: https://codereview.chromium.org/2355593002
|
|
Replace the CPDF_Stream(nullptr, 0, nullptr) pattern with
a default ctor.
Remove unused parameters from CPDF_Stream::SetData(). Both
are always passed as FALSE.
CPDF_Stream declared its own m_GenNum, which shadowed the one
in CPDF_Object. It was used only to distinguish file/memory
streams, so add a bool explicitly for this purpose.
Remove the union, it would be sad if we confused user data
with a C++ object with virtual function calls.
Use unique_ptrs with appropriate deleters to manage memory.
Review-Url: https://codereview.chromium.org/2347993002
|
|
https://codereview.chromium.org/2158023002/ )
Reason for revert:
Causes heap-use-after-free. See crbug.com/647612.
Original issue's description:
> Fix memory leaking on ClosePage.
> CFX_FontCache refactoring:
> after this CL: Only one global CFX_FontCache used. Any cached items from it, are released, when its are not used.
>
> 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
>
> Committed: https://pdfium.googlesource.com/pdfium/+/cde5101eb15b24519e89fa500fe37038bc8e2201
TBR=tsepez@chromium.org,brucedawson@chromium.org,npm@chromium.org,art-snake@yandex-team.ru
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=79367,48791
Review-Url: https://codereview.chromium.org/2350763002
|
|
PWL_FontMap does not need its own charset definitions. fx_edit.h does
not need to define DEFAULT_CHARSET. XFA have their own definitions.
They look different in that most are MAC or MSWin charset definitions.
So they are left untouched. public/fpdf_sysfontinfo.h duplicate ones
were left untouched due to being in public folder.
Review-Url: https://codereview.chromium.org/2347313002
|
|
ProcessbCJK and CalculateFontDesc methods are used to reduce the code
duplication between AddFont and AddWindowsFont methods.
Review-Url: https://codereview.chromium.org/2341373003
|
|
CFX_FontCache refactoring:
after this CL: Only one global CFX_FontCache used. Any cached items from it, are released, when its are not used.
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
Review-Url: https://codereview.chromium.org/2158023002
|
|
This Cl makes the Get and Set methods consistenly use {G|S}et<Type>For.
BUG=pdfium:596
Review-Url: https://codereview.chromium.org/2334323005
|
|
- Methods GetPagesDict, ProcessNonbCJK, CalculateFlags, and
CalculateEncodingDict created to reduce duplicated code.
- Code nits
Review-Url: https://codereview.chromium.org/2323793003
|
|
Review-Url: https://codereview.chromium.org/2323933002
|
|
Fix up callers from CPDF_DataAvail.
Review-Url: https://codereview.chromium.org/2294383003
|
|
Return false instead of crashing.
BUG=641882
Review-Url: https://codereview.chromium.org/2300903002
|
|
BUG=637119
Review-Url: https://codereview.chromium.org/2305443003
|
|
BUG=642655
Review-Url: https://codereview.chromium.org/2298753003
|
|
Currently when the parser utility classes are outputting to a text buffer we do
not verify that an element from an array exists before accessing. We can have
null items in arrays (and dictionaries but the dictionary case is already
handled).
This Cl updates the code to check the element exists before attempting to use
the element.
BUG=chromium:641076
Review-Url: https://codereview.chromium.org/2292473004
|
|
This CL is a speculative fix for the associated BUG. Make sure the CPDF_Document
is initialized in the constructor.
BUG=chromium:640998
Review-Url: https://codereview.chromium.org/2291743002
|
|
BUG=641444
Review-Url: https://codereview.chromium.org/2283893003
|
|
BUG=597440
Review-Url: https://codereview.chromium.org/2273293003
|
|
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 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
|
|
This Cl moves the parser out of the indirect object holder and into the
CPDF_Document where it is used.
Review-Url: https://codereview.chromium.org/2277433003
|
|
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
|
|
This CL moves the m_IndirectObjs map to be private to the IndirectObjectHolder.
Various bits of code have been updated to use the accessors to the map.
This CL fixes the issue with the last time this landed by removing the objnum
check from GetIndirectObject() which appears to have caused the crashes.
Review-Url: https://codereview.chromium.org/2275703002
|
|
This Cl updates the names of the methods in the indirect object holder to better
reflect their usage. The m_LastObjNum is made private and a setter added.
Review-Url: https://codereview.chromium.org/2275593002
|
|
BUG=637119
Review-Url: https://codereview.chromium.org/2274723002
|
|
BUG=637119
Review-Url: https://codereview.chromium.org/2268693003
|
|
https://codereview.chromium.org/2253723002/ )
Reason for revert:
Causing asan issues. See crbug.com/639451.
Original issue's description:
> Move parser pointer to CPDF_Document
>
> The CPDF_IndirectObjectHolder has two subclasses, CPDF_Document and
> CFDF_Document. The CPDF document requires the parser and the CFDF document
> does not. This cl moves the parser pointer up to CPDF_Document.
>
> Committed: https://pdfium.googlesource.com/pdfium/+/260f5fbf3553a96fa49b029cc050220039c30e2a
TBR=tsepez@chromium.org,thestig@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
Review-Url: https://codereview.chromium.org/2266033002
|
|
BUG=636559
Review-Url: https://codereview.chromium.org/2255083004
|
|
Review-Url: https://codereview.chromium.org/2260533002
|
|
Moved classes CFX_FontCache and CFX_AutoFontCache into a separate file.
Review-Url: https://codereview.chromium.org/2246223002
|
|
The CPDF_IndirectObjectHolder has two subclasses, CPDF_Document and
CFDF_Document. The CPDF document requires the parser and the CFDF document
does not. This cl moves the parser pointer up to CPDF_Document.
Review-Url: https://codereview.chromium.org/2253723002
|
|
Review-Url: https://codereview.chromium.org/2247073005
|
|
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
|
|
Review-Url: https://codereview.chromium.org/2241153002
|
|
Even 39 bits is very generous for the number of bits needed to represent
the greatest number of shared object references.
BUG=637119
Review-Url: https://codereview.chromium.org/2242723002
|
|
CPDF_HintTables::ReadSharedObjHintTable() unnecessarily constraints
a FX_FILESIZE value to an int32_t. Relax this check, since the result
will be stored in |m_szSharedObjOffsetArray| which is of FX_FILESIZE.
Bad values in |m_szSharedObjOffsetArray| will still cause hint table
loading to eventually fail.
BUG=635565
Review-Url: https://codereview.chromium.org/2230883003
|
|
- Return earlier when possible.
- Fail rather than crash on invalid values.
Review-Url: https://codereview.chromium.org/2235843002
|
|
Using IsEmpty() is more readable than using GetCount() == 0.
Review-Url: https://codereview.chromium.org/2226113002
|
|
This CL fixes up the crypto key copying code to better handle big endian
machines.
BUG=pdfium:147
Review-Url: https://codereview.chromium.org/2190123002
|