Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(123)

Issue 2428743004: Make Document::m_IconList a vector of IconElements. (Closed)

Created:
4 years, 2 months ago by Lei Zhang
Modified:
4 years, 2 months ago
Reviewers:
npm, dsinclair
CC:
pdfium-reviews_googlegroups.com
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Make Document::m_IconList a vector of IconElements. There's no need for std::list<std::unique_ptr<IconElement>>. Committed: https://pdfium.googlesource.com/pdfium/+/f328d0d378b8df8a3416988d96c34f1d3f9d26d1

Patch Set 1 #

Total comments: 4

Patch Set 2 : nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -33 lines) Patch
M fpdfsdk/javascript/Document.h View 5 chunks +7 lines, -6 lines 0 comments Download
M fpdfsdk/javascript/Document.cpp View 1 5 chunks +25 lines, -27 lines 0 comments Download

Messages

Total messages: 14 (8 generated)
Lei Zhang
4 years, 2 months ago (2016-10-18 19:28:24 UTC) #6
npm
lgtm https://codereview.chromium.org/2428743004/diff/1/fpdfsdk/javascript/Document.cpp File fpdfsdk/javascript/Document.cpp (right): https://codereview.chromium.org/2428743004/diff/1/fpdfsdk/javascript/Document.cpp#newcode1323 fpdfsdk/javascript/Document.cpp:1323: Icon* pRetIcon = element.IconStream; Remove https://codereview.chromium.org/2428743004/diff/1/fpdfsdk/javascript/Document.cpp#newcode1340 fpdfsdk/javascript/Document.cpp:1340: pIcon->SetStream(pRetIcon->GetStream()); ...
4 years, 2 months ago (2016-10-18 19:34:45 UTC) #7
Lei Zhang
https://codereview.chromium.org/2428743004/diff/1/fpdfsdk/javascript/Document.cpp File fpdfsdk/javascript/Document.cpp (right): https://codereview.chromium.org/2428743004/diff/1/fpdfsdk/javascript/Document.cpp#newcode1323 fpdfsdk/javascript/Document.cpp:1323: Icon* pRetIcon = element.IconStream; On 2016/10/18 19:34:45, npm wrote: ...
4 years, 2 months ago (2016-10-18 22:28:44 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2428743004/20001
4 years, 2 months ago (2016-10-18 22:28:52 UTC) #11
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://pdfium.googlesource.com/pdfium/+/f328d0d378b8df8a3416988d96c34f1d3f9d26d1
4 years, 2 months ago (2016-10-18 22:38:26 UTC) #13
dsinclair
4 years, 2 months ago (2016-10-19 02:26:19 UTC) #14
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2431913003/ by dsinclair@chromium.org.

The reason for reverting is: Appears to be blocking the roll due to compile
failure https://codereview.chromium.org/2429053002

Attempting to revert to see if the roll will pass..

Powered by Google App Engine
This is Rietveld 408576698