Age | Commit message (Collapse) | Author |
|
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
|
|
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
|
|
The prefix doesn't add anything when used in CPDFSDK_Environment, remove.
Review-Url: https://codereview.chromium.org/2338303002
|
|
[DO NOT COMMIT]
This CL renames IXFA_DocProvider to IXFA_DocEnvironment to better describe the
purpose. Then, CPDFXFA_Document has all of the IXFA_DocEnvironment methods
removed and placed in CPDFXFA_DocEnvironment. The CPDFXFA_Document then
has a CPDFXFA_DocEnvironment.
This splits the code related to the document apart from the XFA callback methods
to work with that document.
Review-Url: https://codereview.chromium.org/2328573002
|
|
Remove unused params and methods. Cleanup formatting.
Review-Url: https://codereview.chromium.org/2322003002
|
|
Remove methods that always just return without doing any work. Clean up the
IXFA_DocProvider interface calls for those methods and cleanup the callers
that were calling the methods.
Review-Url: https://codereview.chromium.org/2323523002
|
|
This CL updates all of the includes to be correctly sorted. A PRESUBMIT warning
is added (from chromium) that will warn if the includes are in the wrong order on upload.
Review-Url: https://codereview.chromium.org/2337293002
|
|
Use CFX_DefStore to only replace CFX_FixedStore, but not
CFX_StaticStore, since CFX_StaticStore has different behaviors.
CFX_StaticStore doesn't require its users to explicitly call free(),
it frees all the allocated memory during destruction. Use
CFX_DefStore to replace CFX_StaticStore would cause leaks.
Also remove two undeclared, but defined, functions.
BUG=pdfium:242
Review-Url: https://codereview.chromium.org/2328403002
|
|
Reland of Fix leaked internal font (patchset #2 id:60001 of
https://codereview.chromium.org/2297303004/ )
In CFGAS_FontMgrImp::LoadFont(), a new internal font is created which
is never released. It needs to be correctly marked as internal font to
be released. Fix this by adding a new method to take the ownership
of the font and mark it as internal font properly.
The previous revert was caused by memory management errors
which were fixed at https://codereview.chromium.org/2322043002/
BUG=pdfium:242
Review-Url: https://codereview.chromium.org/2320213002
|
|
A few issues are fixed:
--Change variable |m_bLogic| in CFX_Font to |m_bShallowCopy| to
reflect its meaning better;
--For a shallow copy of font, we must guarantee that the copied font
will not be deleted until the shallow copy is deleted. So need to
increase the src font's refcount when copying it;
--The stream |m_pOwnedStream| needs to have matched new/delete
These errors need to be fixed before we can properly delete all the
fonts to address the leaks.
BUG=pdfium:242
Review-Url: https://codereview.chromium.org/2322043002
|
|
Two leak cases are addressed here:
--In CFGAS_FontMgrImp::LoadFont(), calling LoadFace() is unnecessary
since the following LoadFile() does the exact same thing. Calling
LoadFace() without releasing the loaded face results in a leak;
--|m_Hash2Fonts| in class CFGAS_FontMgrImp owns all the fonts stored
in it. The fonts need to be deleted along with the container.
BUG=pdfium:242
Review-Url: https://codereview.chromium.org/2322483003
|
|
Review-Url: https://codereview.chromium.org/2318423002
|
|
https://codereview.chromium.org/2297303004/ )
Reason for revert:
asan bot doesn't like it, will investigate
Original issue's description:
> Fix leaked internal font
>
> In CFGAS_FontMgrImp::LoadFont(), a new internal font is created which
> is never released. It needs to be correctly marked as internal font to
> be released. Fix this by adding a boolean parameter and pass it along
> during the creation of the font.
>
> BUG=pdfium:242
>
> Committed: https://pdfium.googlesource.com/pdfium/+/6708106e6a3d54f3370c871ebf6643d1ecf58999
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=pdfium:242
Review-Url: https://codereview.chromium.org/2302213002
|
|
In CFGAS_FontMgrImp::LoadFont(), a new internal font is created which
is never released. It needs to be correctly marked as internal font to
be released. Fix this by adding a boolean parameter and pass it along
during the creation of the font.
BUG=pdfium:242
Review-Url: https://codereview.chromium.org/2297303004
|
|
During XFA text layout, text pieces are allocated with lines, and
their text and widths are also allocated. However, they are not
freed in any place. Fix this by releasing them along with lines
during unload() process.
BUG=pdfium:242
Review-Url: https://codereview.chromium.org/2297563006
|
|
The blocks.Add will std::move the unique_ptrs, so the std::max calculations
need to go before. Without this change, pdfium_test will crash when trying
to render test/barcode_test.pdf with XFA enabled.
Tested that after this change, barcode_test.pdf is rendered without crashing.
Review-Url: https://codereview.chromium.org/2298833002
|
|
Review-Url: https://codereview.chromium.org/2292503002
|
|
The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.
The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.
BUG=chromium:637546
Review-Url: https://codereview.chromium.org/2259823004
|
|
Change the places which require implicit construction, and make the
construction from ARGB_Color explicit.
Review-Url: https://codereview.chromium.org/2263923003
|
|
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
|
|
Return unique_ptr for GetLocale(), directly use destructors
instead of Release() functions, use vectors to manage arrays.
Review-Url: https://codereview.chromium.org/2241863002
|
|
The two methods in fgas_system also exist in core/fxcrt/include/fx_ext with the
FXSYS_ prefix instead of FX_. Remove the fgas_system files and use the
fx_ext versions instead.
Review-Url: https://codereview.chromium.org/2233133002
|
|
This Cl fixes the CFDE_XMLSyntaxParser::ParseTextChar() to handle entities
where the value goes negative. Currently this could cause an undefined-shift
as due to the (ch << 4) calls. Instead, detect if the value has gone negative
and return a space character.
BUG=chromium:603489
Review-Url: https://codereview.chromium.org/2223823003
|
|
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
|
|
Use smart pointers instead of raw pointer to make memory management
easier for classes under xfa/fwl/theme.
BUG=pdfium:518
Review-Url: https://codereview.chromium.org/2230813002
|
|
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
|
|
For classes under xfa/fxbarcode, use smart pointers instead
of raw pointer to make memory management easier.
Also fix some styling issues along the changes.
BUG=pdfium:518
Review-Url: https://codereview.chromium.org/2221023003
|
|
Review-Url: https://codereview.chromium.org/2226003003
|
|
For classes under xfa/fxfa/fm2js, and xfa/fxgraphics, use smart
pointers instead of raw pointer to make memory management easier.
Also fix some styling issues along the changes.
BUG=pdfium:518
Review-Url: https://codereview.chromium.org/2222203002
|
|
Use virtual function to return the actual interface type instead
of the base interface type to avoid a lot of casts.
Also tidy up CFWL_Widget by encapsulating variables, and use
smart pointers for class owned member variables.
Review-Url: https://codereview.chromium.org/2209153002
|
|
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
|
|
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
|
|
For classes under xfa/fgas, xfa/fwl/basewidget, and xfa/fwl/core,
use smart pointers instead of raw pointer to make memory management
easier.
BUG=pdfium:518
Review-Url: https://codereview.chromium.org/2207093005
|
|
Use smart pointer to replace raw pointer type for class
owned member variables so that memory management will
be easier.
BUG=pdfium:518
Review-Url: https://codereview.chromium.org/2208423002
|
|
Fix a file name to be consistent with its class name and header file
name.
Review-Url: https://codereview.chromium.org/2215813003
|
|
This renames the file to match the class name.
Review-Url: https://codereview.chromium.org/2209823002
|
|
Fix CXFA_FMIdentifierExpressionn to remove the duplicate n.
Review-Url: https://codereview.chromium.org/2210543002
|
|
This moves the needed traverse strategies into their own files, removes the
unused one and cleans up the includes.
Review-Url: https://codereview.chromium.org/2207033002
|
|
When determining which params should be an object and which are a value it is
possible to overflow the int on the shift comparision (if there are more then
32 arguments).
This never happens in practise as it's a controlled list of method calls which
we pass objects for. Cap the check at 32 for the shifting so it doesn't
overflow. We can revisit and extend the value later if we ever have an internal
formcalc method that needs an object in a position greater then 32.
BUG=chromium:603490
Review-Url: https://codereview.chromium.org/2206253002
|
|
This CL splits the header file apart. The cpp files are not touched as part
of this CL, they will be done as a followup. This de-duplicates the fpdf_doc.h
BUG=pdfium:249
Review-Url: https://codereview.chromium.org/2183313004
|
|
Move CFX_FolderFontInfo, CFX_FontMgr, and CFX_FontMapper into their own
classes. There are namespaces in each of the new files, having methods
from the original namespace in fx_ge_fontmap, according to what each
class needs.
Review-Url: https://codereview.chromium.org/2185533006
|
|
BUG=pdfium:112
TBR=dsinclair@chromium.org
Review-Url: https://codereview.chromium.org/2183703004
|
|
For the class owned member variables, use std::unique_ptr or
std::vector for memory management.
BUG=pdfium:518
Review-Url: https://codereview.chromium.org/2169793002
|
|
This CL removes default params from CXFA_SimpleParser and updates the call sites
as necessary.
Review-Url: https://codereview.chromium.org/2164963003
|
|
This CL moves the |SetFactory| method to be public and removes the friendship
with CXFA_DocumentParser from CXFA_SimpleParser.
Review-Url: https://codereview.chromium.org/2162263003
|
|
This no longer seems to be needed (removing causes no issues).
Review-Url: https://codereview.chromium.org/2168483004
|
|
This makes the cpp and unittest files match the naming of the header file.
Review-Url: https://codereview.chromium.org/2165833005
|
|
This CL moves CXFA_DataImporter and CXFA_DataExporter into their own files
and moves code to an anonymous namespace as necessary.
Review-Url: https://codereview.chromium.org/2164663004
|