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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1887703003: Fold the FWL NoteThread classes up to the Thread classes. (Closed)

Created:
4 years, 8 months ago by dsinclair
Modified:
4 years, 8 months 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

Fold the FWL NoteThread classes up to the Thread classes. The NoteThread classes only contained one member and don't provide any added clarity. This CL moves the NoteDriver up to the Thread classes and removes the NoteThread classes. Committed: https://pdfium.googlesource.com/pdfium/+/7adef41ed27ced7e18ee649a3d4f27a3cc00c51f

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Patch Set 3 : #

Total comments: 2

Patch Set 4 : #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+61 lines, -101 lines) Patch
M BUILD.gn View 1 1 chunk +0 lines, -1 line 0 comments Download
M xfa.gyp View 1 1 chunk +0 lines, -1 line 0 comments Download
M xfa/fwl/basewidget/fwl_comboboximp.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
M xfa/fwl/core/fwl_appimp.h View 1 chunk +1 line, -1 line 0 comments Download
M xfa/fwl/core/fwl_appimp.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M xfa/fwl/core/fwl_formimp.cpp View 1 6 chunks +6 lines, -6 lines 0 comments Download
M xfa/fwl/core/fwl_noteimp.h View 1 chunk +1 line, -1 line 0 comments Download
M xfa/fwl/core/fwl_noteimp.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M xfa/fwl/core/fwl_threadimp.h View 1 2 3 1 chunk +8 lines, -14 lines 1 comment Download
M xfa/fwl/core/fwl_threadimp.cpp View 1 1 chunk +3 lines, -14 lines 0 comments Download
M xfa/fwl/core/fwl_widgetimp.h View 3 chunks +5 lines, -5 lines 0 comments Download
M xfa/fwl/core/fwl_widgetimp.cpp View 1 9 chunks +11 lines, -11 lines 0 comments Download
M xfa/fwl/core/fwl_widgetmgrimp.cpp View 1 chunk +3 lines, -3 lines 0 comments Download
M xfa/fwl/core/ifwl_app.h View 2 chunks +2 lines, -2 lines 0 comments Download
M xfa/fwl/core/ifwl_notedriver.h View 2 chunks +2 lines, -2 lines 0 comments Download
D xfa/fwl/core/ifwl_notethread.h View 1 chunk +0 lines, -19 lines 0 comments Download
M xfa/fwl/core/ifwl_thread.h View 1 2 2 chunks +3 lines, -5 lines 0 comments Download
M xfa/fwl/core/ifwl_widget.h View 2 chunks +4 lines, -4 lines 0 comments Download
M xfa/fwl/lightwidget/cfwl_widget.cpp View 5 chunks +5 lines, -5 lines 0 comments Download

Messages

Total messages: 12 (3 generated)
dsinclair
PTAL.
4 years, 8 months ago (2016-04-13 20:39:55 UTC) #2
Tom Sepez
https://codereview.chromium.org/1887703003/diff/1/xfa/fwl/core/ifwl_thread.h File xfa/fwl/core/ifwl_thread.h (right): https://codereview.chromium.org/1887703003/diff/1/xfa/fwl/core/ifwl_thread.h#newcode34 xfa/fwl/core/ifwl_thread.h:34: IFWL_NoteDriver* GetNoteDriver(); is this a const method?
4 years, 8 months ago (2016-04-13 21:45:29 UTC) #3
dsinclair
https://codereview.chromium.org/1887703003/diff/1/xfa/fwl/core/ifwl_thread.h File xfa/fwl/core/ifwl_thread.h (right): https://codereview.chromium.org/1887703003/diff/1/xfa/fwl/core/ifwl_thread.h#newcode34 xfa/fwl/core/ifwl_thread.h:34: IFWL_NoteDriver* GetNoteDriver(); On 2016/04/13 21:45:29, Tom Sepez wrote: > ...
4 years, 8 months ago (2016-04-14 13:23:36 UTC) #4
Tom Sepez
https://codereview.chromium.org/1887703003/diff/40001/xfa/fwl/core/fwl_threadimp.h File xfa/fwl/core/fwl_threadimp.h (right): https://codereview.chromium.org/1887703003/diff/40001/xfa/fwl/core/fwl_threadimp.h#newcode20 xfa/fwl/core/fwl_threadimp.h:20: virtual ~CFWL_ThreadImp() {} so do we just leak the ...
4 years, 8 months ago (2016-04-14 16:01:59 UTC) #5
dsinclair
https://codereview.chromium.org/1887703003/diff/40001/xfa/fwl/core/fwl_threadimp.h File xfa/fwl/core/fwl_threadimp.h (right): https://codereview.chromium.org/1887703003/diff/40001/xfa/fwl/core/fwl_threadimp.h#newcode20 xfa/fwl/core/fwl_threadimp.h:20: virtual ~CFWL_ThreadImp() {} On 2016/04/14 16:01:59, Tom Sepez wrote: ...
4 years, 8 months ago (2016-04-14 16:52:53 UTC) #6
Tom Sepez
lgtm https://codereview.chromium.org/1887703003/diff/60001/xfa/fwl/core/fwl_threadimp.h File xfa/fwl/core/fwl_threadimp.h (right): https://codereview.chromium.org/1887703003/diff/60001/xfa/fwl/core/fwl_threadimp.h#newcode21 xfa/fwl/core/fwl_threadimp.h:21: virtual ~CFWL_ThreadImp() { delete m_pNoteDriver; } nit: virtual ...
4 years, 8 months ago (2016-04-14 21:20:55 UTC) #7
dsinclair
On 2016/04/14 21:20:55, Tom Sepez wrote: > lgtm > > https://codereview.chromium.org/1887703003/diff/60001/xfa/fwl/core/fwl_threadimp.h > File xfa/fwl/core/fwl_threadimp.h (right): ...
4 years, 8 months ago (2016-04-18 13:32:15 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1887703003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1887703003/60001
4 years, 8 months ago (2016-04-18 13:32:21 UTC) #10
commit-bot: I haz the power
4 years, 8 months ago (2016-04-18 13:49:07 UTC) #12
Message was sent while issue was closed. 
Committed patchset #4 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/7adef41ed27ced7e18ee649a3d4f27a3cc00...

Powered by Google App Engine
This is Rietveld 408576698