Age | Commit message (Collapse) | Author |
|
Try 2: main fix was recursively popping elements from the stack. Since
the Traverse method can be called on non-root nodes from GetPage(), we
have to make sure to properly update the parents.
Try 1 at https://codereview.chromium.org/2414423002/
In our current implementation of CPDF_Document::GetPage, we traverse
the PDF page tree until we find the index we are looking for. This is
slow when we do calls GetPage(0), GetPage(1), ... since in this case
the page tree will be traversed n times if there are n pages. This CL
makes sure the page tree is only traversed once.
Time to load the PDF from the bug below in chrome official build:
Before this CL: around 1 minute 25 seconds
After this CL: around 4 seconds
BUG=chromium:638513
Review-Url: https://codereview.chromium.org/2442403002
|
|
Found by winxfa bot when fx_bool defined to bool.
Review-Url: https://codereview.chromium.org/2449293002
|
|
It's been troubling for some time that an IFX_FileStream might
actually be an in-memory buffer with no backing file.
Review-Url: https://codereview.chromium.org/2443723002
|
|
The CPDF_Document does not own its root dictionary, so add ownership in
CPDF_TestDocumentForPages, using ReleaseDeleter because the dictionary
cannot be deleted.
R=tsepez@chromium.org
Review URL: https://codereview.chromium.org/2445753002 .
|
|
Added a nontrivial page tree and a test that pages are being fetched
properly, both when requested in order and in reverse order. This will
help prevent introducing bugs while changing the way the page tree is
processed.
BUG=chromium:638513
Review-Url: https://chromiumcodereview.appspot.com/2435783006
|
|
Review-Url: https://chromiumcodereview.appspot.com/2424933002
|
|
https://codereview.chromium.org/2437773003/ )
Reason for revert:
CPDF_DataAvail::IsPageAvail is causing crashes.
BUG=chromium:658168, chromium:658170
Original issue's description:
> Fix loading page using hint tables.
>
> When linearized document have hint table,
> The FPDFAvail_IsPageAvail return true, but
> FPDF_LoadPage return nullptr, for non first pages.
>
> This happens, bacause document not use hint tables, to load page.
>
> To fix this, I force save the page's ObjNum in document.
>
> R=npm, dsinclair
>
> Committed: https://pdfium.googlesource.com/pdfium/+/ef38283688c1ee7c08bcf4204cfb78e09c039782
TBR=dsinclair@chromium.org,tsepez@chromium.org,thestig@chromium.org,art-snake@yandex-team.ru
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://chromiumcodereview.appspot.com/2442663005
|
|
When linearized document have hint table,
The FPDFAvail_IsPageAvail return true, but
FPDF_LoadPage return nullptr, for non first pages.
This happens, bacause document not use hint tables, to load page.
To fix this, I force save the page's ObjNum in document.
R=npm, dsinclair
Review-Url: https://chromiumcodereview.appspot.com/2437773003
|
|
id:60001 of https://codereview.chromium.org/2414423002/ )
Reason for revert:
Possible cause of crbug.com/657897 reverting to find out.
BUG=657897
Original issue's description:
> Traverse PDF page tree only once in CPDF_Document
>
> In our current implementation of CPDF_Document::GetPage, we traverse
> the PDF page tree until we find the index we are looking for. This is
> slow when we do calls GetPage(0), GetPage(1), ... since in this case
> the page tree will be traversed n times if there are n pages. This CL
> makes sure the page tree is only traversed once.
>
> Time to load the PDF from the bug below in chrome official build:
> Before this CL: 1 minute 40 seconds
> After this CL: 5 seconds
>
> BUG=chromium:638513
>
> Committed: https://pdfium.googlesource.com/pdfium/+/7c29e27dae139a205755c1a29b7f3ac8b36ec0da
TBR=thestig@chromium.org,tsepez@chromium.org,npm@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=chromium:638513
Review-Url: https://chromiumcodereview.appspot.com/2430313006
|
|
This Cl merges the implementation classes up into the IFWL classes as the
intermediate classes were just acting as proxies.
Review-Url: https://chromiumcodereview.appspot.com/2432423002
|
|
In our current implementation of CPDF_Document::GetPage, we traverse
the PDF page tree until we find the index we are looking for. This is
slow when we do calls GetPage(0), GetPage(1), ... since in this case
the page tree will be traversed n times if there are n pages. This CL
makes sure the page tree is only traversed once.
Time to load the PDF from the bug below in chrome official build:
Before this CL: 1 minute 40 seconds
After this CL: 5 seconds
BUG=chromium:638513
Review-Url: https://codereview.chromium.org/2414423002
|
|
The assumption that only indirect objects would be passed is wrong.
Restore old behaviour despite muddying ownership constraints.
R=thestig@chromium.org
BUG=656145
Review-Url: https://codereview.chromium.org/2425843002
|
|
Avoid confusing this class with other ref-counted objects.
Review-Url: https://codereview.chromium.org/2426673002
|
|
This reverts commit 1d023881cd53485303c0fcc0b5878e700dc470fd.
Reason for revert -- fuzzers hit issues.
TBR=thestig@chromium.org
Review-Url: https://codereview.chromium.org/2425783002
|
|
This reverts commit 3ba098595ae56b64eacc0c25ab76b89a4d78d920.
TBR=thestig@chromium.org,weili@chromium.org
Review URL: https://codereview.chromium.org/2424533003 .
|
|
Doing so highlights a few places where ownership is dubious.
Add convenience functions to return an unowned reference to
a newly-created indirect object.
Review-Url: https://codereview.chromium.org/2419173002
|
|
This tweaks the implementation while leaving the API the
same. The API change is more disruptive, so break this
part off first.
Review-Url: https://codereview.chromium.org/2385293002
|
|
Also move code that belongs in an anonymous namespace into one.
Review-Url: https://codereview.chromium.org/2416703002
|
|
Introduce the UniqueDictionary typedef and friends, to allow
moving to unique_ptrs before the Release() deleter issue is
fully resolved. This will go away down the road.
Review-Url: https://codereview.chromium.org/2420743002
|
|
No functional changes.
Make many of its methods private.
Review-Url: https://codereview.chromium.org/2415943003
|
|
Small step before making CPDF_Image stream ownership saner.
Review-Url: https://codereview.chromium.org/2416733002
|
|
The CPDF_Page::GetDisplayMatrix expects to set float values into the
|display_matrix| but all of the input values are currently int. It is possible
to overflow the int values, so this CL changes the variables to be int which
closer reflects what they're being used for.
BUG=chromium:652038
Review-Url: https://codereview.chromium.org/2412983002
|
|
Added methods SetExtGID and CalcExtGID for this purpose.
Changed protected to private: no one inherits CPDF_Type1Font.
Review-Url: https://codereview.chromium.org/2412483006
|
|
BUG=chromium:639792
Review-Url: https://codereview.chromium.org/2415483002
|
|
They won't work with std::unique_ptr down the road, so replace
with std::vector.
Review-Url: https://codereview.chromium.org/2411703003
|
|
Rolling 0 times is a no-op.
Rolling 0 items is a no-op.
Rolling N items J times is the same as rolling N items J % N times.
This also avoids an integer overflow corner case.
BUG=chromium:648077
Review-Url: https://codereview.chromium.org/2412833002
|
|
Review-Url: https://codereview.chromium.org/2409173003
|
|
Prevent having to remember what an object number of 0 implies.
Review-Url: https://codereview.chromium.org/2412673002
|
|
Review-Url: https://codereview.chromium.org/2409283002
|
|
Ditto with the other ToXXX functions to CPDF_xxx.h.
Shortly, we will want to introduce another variant:
inline std::unique_ptr<CPDF_String> ToString(
std::unique_ptr<CPDF_Object>);
This will require that CPDF_String be complete which is
not the case in the CPDF_Object.h header. Rather than
dragging all the other .h's into it, move these to the
subclass headers. That way, these will be together
when the new form is added.
Review-Url: https://codereview.chromium.org/2406033002
|
|
Split this off so that we don't keep losing this when
the assert is reverted again.
Review-Url: https://codereview.chromium.org/2401423005
|
|
Avoid an assert which previously could only be overcome
by removing/re-inserting.
Back-fill a unit test for the equivalent Array method.
BUG=654387
Review-Url: https://codereview.chromium.org/2403143002
|
|
This reverts commit b69a98cf50537130f88ce3a799117f2ca8353ac5.
Reason for revert: crashes on mac.
BUG=654387
TBR=thestig@chromium.org
Review-Url: https://codereview.chromium.org/2410483002
|
|
- Remove some unused stuff from pageint.h.
- Replace some FX_BOOL with bool in pageint.h, and related.
- Replace some "protected" with "private" in pageint.h.
- Move 2 methods into namespace in fpdf_page_parser_old.cpp.
Review-Url: https://codereview.chromium.org/2399573002
|
|
This reverts commit b73c99335bfbd158ad16dd59c9c52396ffd2b54b.
TBR=thestig@chromium.org
Review-Url: https://codereview.chromium.org/2393783004
|
|
This avoids a re-assignment that can otherwise cause a later fault.
BUG=pdfium:607
Review-Url: https://codereview.chromium.org/2393953002
|
|
FX_BOOL can be replaced by bool. Also replaced in a couple other places
so that Winbots pass.
Review-Url: https://codereview.chromium.org/2395803002
|
|
There's no way to take ownership back from the CPDF_Array
without deleting the object, so add a new primitive to make
elements become indirect rather than manipulating them
outside the class.
This should solve the ASSERT(objnum == 0) issue that
blocked the previous roll.
Review-Url: https://codereview.chromium.org/2391883003
|
|
- Added private method to avoid duplicated code.
- If the unicode calculation overflows, 0 is used instead of crashing.
Review-Url: https://codereview.chromium.org/2392103002
|
|
BUG=pdfium:603
Review-Url: https://codereview.chromium.org/2393593002
|
|
BUG=pdfium:603
Review-Url: https://codereview.chromium.org/2392603004
|
|
BUG=pdfium:603
Review-Url: https://codereview.chromium.org/2386423004
|
|
BUG=pdfium:603
Review-Url: https://codereview.chromium.org/2392773003
|
|
BUG=pdfium:603
Review-Url: https://codereview.chromium.org/2386263003
|
|
BUG=pdfium:603
Review-Url: https://codereview.chromium.org/2391013002
|
|
m_Map maps to unsigned integer, but m_MultiCharBuf.GetLength() returns
an integer. There will be integer overflow if the length is big, and
UBSAN will complain. Thus, using FX_SAFE_UINT32. Replacing with uint32
would work as well: the point is to consider the length as uint instead
of int.
BUG=chromium:652232
Review-Url: https://codereview.chromium.org/2393573002
|
|
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
|