Chromium Code Reviews
Help | Chromium Project | Sign in
(13)

Issue 2202283002: Introduce ::getPropertyInternal method to avoid duplication (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months, 3 weeks ago by tonikitoo
Modified:
7 months, 2 weeks ago
Reviewers:
Tom Sepez, Lei Zhang
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@master
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Introduce ::getPropertyInternal method to avoid duplication Document.cpp implements "getter" methods for some of the Document object properties. Some of the body of such methods are identical. Patch introduces a ::getPropertyInternal private method that gets rid of this duplication. Namely the following properties' getters are cleaned up: - "author", "creationDate", "creator", "keywords", - "modDate", "producer", "subject" and "title" No behavior change. Committed: https://pdfium.googlesource.com/pdfium/+/ea3ff9eaaa508b9cbc7f52bc92d189eacbc7a935

Patch Set 1 #

Patch Set 2 : Same as patchset 1, but also added myself to AUTHORS #

Patch Set 3 : Add support to Document::URL property getter. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -0 lines) Patch
M fpdfsdk/javascript/Document.h View 1 2 2 chunks +2 lines, -0 lines 0 comments Download
M fpdfsdk/javascript/Document.cpp View 1 2 2 chunks +14 lines, -0 lines 0 comments Download
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 10 (3 generated)
tonikitoo
7 months, 3 weeks ago (2016-08-02 15:37:03 UTC) #2
Tom Sepez
On 2016/08/02 15:37:03, tonikitoo1 wrote: Hi tonikitoo1, thanks for the patch. It is a lot ...
7 months, 3 weeks ago (2016-08-02 17:42:26 UTC) #3
tonikitoo
On 2016/08/02 17:42:26, Tom Sepez wrote: > On 2016/08/02 15:37:03, tonikitoo1 wrote: > > Hi ...
7 months, 3 weeks ago (2016-08-02 17:49:41 UTC) #4
Tom Sepez
>I have signed Chromium's contributor agreement in src/AUTHORS. > I actually fall under Igalia's corporate ...
7 months, 3 weeks ago (2016-08-02 17:52:24 UTC) #5
Tom Sepez
lgtm
7 months, 3 weeks ago (2016-08-02 18:24:07 UTC) #7
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/2202283002/20001
7 months, 3 weeks ago (2016-08-02 18:24:10 UTC) #8
commit-bot: I haz the power
7 months, 3 weeks ago (2016-08-02 18:52:31 UTC) #10
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://pdfium.googlesource.com/pdfium/+/ea3ff9eaaa508b9cbc7f52bc92d189eacbc7...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld d1a128a62