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 79933003: Restructuring of PdfViewer code. (Closed)

Created:
7 years, 1 month ago by scroggo
Modified:
7 years ago
Reviewers:
mtklein
CC:
skia-review_googlegroups.com
Visibility:
Public.

Description

Restructuring of PdfViewer code. The only change in behavior is that SkPdfAllocator on SkPdfContext is no longer allocated on the heap. In general, I have just moved code. SkPdfContext: Inherit from SkNoncopyable. Make SkPdfContext directly own fTmpPageAllocator. fTmpPageAllocator is created when SkPdfContext is, and destroyed at the same time as well, so there is no reason for the extra allocation. Add the function parseStream. This eliminates code duplication, and allows making fTmpPageAllocator private. Move PdfMainLooper into the implementation file, since it is now only used by parseStream. Move SkTDictWithDefaultConstructor and render stats info here, in support of PdfMainLooper. SkPdfTokenLooper: Rename PdfTokenLooper to SkPdfTokenLooper. Move readToken here, unchanged. Remove tokenizer(), which is unused. SkPdfNativeDoc: Remove tokenizerOfPage and tokenizerOfBuffer, which are unused. SkPdfOps: Move gPdfOps and PdfOperatorRenderer into a header file (hidden for now), so they can be accessed by both SkPdfRenderer.cpp and SkPdfContext.cpp. SkPdfRenderer: Harvest things into other files: PdfMainLooper (and the code that calls it) -> SkPdfContext. readToken -> SkPdfTokenLooper. R=mtklein@google.com Committed: https://code.google.com/p/skia/source/detail?r=12435

Patch Set 1 #

Patch Set 2 : #

Total comments: 11

Patch Set 3 : Respond to comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+425 lines, -372 lines) Patch
M experimental/PdfViewer/inc/SkPdfContext.h View 1 2 1 chunk +22 lines, -8 lines 0 comments Download
M experimental/PdfViewer/inc/SkPdfTokenLooper.h View 1 chunk +16 lines, -8 lines 0 comments Download
M experimental/PdfViewer/pdfparser/native/SkPdfNativeDoc.h View 1 chunk +0 lines, -10 lines 0 comments Download
M experimental/PdfViewer/pdfparser/native/SkPdfNativeDoc.cpp View 2 chunks +0 lines, -15 lines 0 comments Download
M experimental/PdfViewer/src/SkPdfContext.cpp View 1 2 1 chunk +121 lines, -4 lines 0 comments Download
A experimental/PdfViewer/src/SkPdfOps.h View 1 chunk +18 lines, -0 lines 0 comments Download
M experimental/PdfViewer/src/SkPdfRenderer.cpp View 1 63 chunks +91 lines, -327 lines 0 comments Download
A experimental/PdfViewer/src/SkPdfTokenLooper.cpp View 1 2 1 chunk +156 lines, -0 lines 0 comments Download
M gyp/pdfviewer_lib.gyp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
scroggo
7 years, 1 month ago (2013-11-21 20:51:57 UTC) #1
mtklein
https://codereview.chromium.org/79933003/diff/30001/experimental/PdfViewer/inc/SkPdfContext.h File experimental/PdfViewer/inc/SkPdfContext.h (right): https://codereview.chromium.org/79933003/diff/30001/experimental/PdfViewer/inc/SkPdfContext.h#newcode35 experimental/PdfViewer/inc/SkPdfContext.h:35: SkPdfContext(SkPdfNativeDoc* doc); explicit? https://codereview.chromium.org/79933003/diff/30001/experimental/PdfViewer/src/SkPdfContext.cpp File experimental/PdfViewer/src/SkPdfContext.cpp (right): https://codereview.chromium.org/79933003/diff/30001/experimental/PdfViewer/src/SkPdfContext.cpp#newcode43 experimental/PdfViewer/src/SkPdfContext.cpp:43: ...
7 years, 1 month ago (2013-11-22 14:40:20 UTC) #2
scroggo
On Fri, Nov 22, 2013 at 9:46 AM, <mtklein@google.com> wrote: > Feel free to ignore ...
7 years, 1 month ago (2013-11-22 15:03:00 UTC) #3
mtklein
On 2013/11/22 15:03:00, scroggo wrote: > On Fri, Nov 22, 2013 at 9:46 AM, <mailto:mtklein@google.com> ...
7 years, 1 month ago (2013-11-22 15:06:21 UTC) #4
scroggo
7 years ago (2013-12-02 20:18:17 UTC) #5
Message was sent while issue was closed.
Committed patchset #3 manually as r12435 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698