Issue 1874963002: Remove some FWL code.

Issue 1874963002: Remove some FWL code. (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.

More Reviews

Description

Remove some FWL code. Remove unused or ifdef'd code from FWL. Committed: https://pdfium.googlesource.com/pdfium/+/380f20a6000bd640d7fd8f1567961f1f05d79f5a

Patch Set 2 : #

Created: 4 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+29 lines, -438 lines)			Patch
M	BUILD.gn	View	1	1 chunk	+0 lines, -2 lines	0 comments	Download
M	xfa.gyp	View	1	1 chunk	+0 lines, -2 lines	0 comments	Download
M	xfa/fwl/basewidget/ifwl_checkbox.h	View	1	1 chunk	+2 lines, -1 line	0 comments	Download
M	xfa/fwl/basewidget/ifwl_combobox.h	View	1	2 chunks	+1 line, -13 lines	0 comments	Download
M	xfa/fwl/basewidget/ifwl_datetimepicker.h	View	1	1 chunk	+1 line, -5 lines	0 comments	Download
M	xfa/fwl/basewidget/ifwl_edit.h	View	1	1 chunk	+1 line, -6 lines	0 comments	Download
M	xfa/fwl/basewidget/ifwl_listbox.h	View	1	1 chunk	+1 line, -2 lines	0 comments	Download
M	xfa/fwl/basewidget/ifwl_monthcalendar.h	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	xfa/fwl/basewidget/ifwl_spinbutton.h	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	xfa/fwl/core/cfwl_event.h	View	1	1 chunk	+1 line, -13 lines	0 comments	Download
M	xfa/fwl/core/fwl_formimp.h	View		1 chunk	+3 lines, -8 lines	0 comments	Download
M	xfa/fwl/core/fwl_sdadapterimp.h	View		1 chunk	+0 lines, -7 lines	0 comments	Download
M	xfa/fwl/core/fwl_sdadapterimp.cpp	View		1 chunk	+3 lines, -17 lines	0 comments	Download
M	xfa/fwl/core/fwl_threadimp.h	View		2 chunks	+0 lines, -2 lines	0 comments	Download
M	xfa/fwl/core/fwl_threadimp.cpp	View		1 chunk	+3 lines, -11 lines	0 comments	Download
M	xfa/fwl/core/ifwl_adapterthreadmgr.h	View		1 chunk	+1 line, -7 lines	0 comments	Download
M	xfa/fwl/core/ifwl_form.h	View		1 chunk	+0 lines, -11 lines	0 comments	Download
M	xfa/fwl/core/ifwl_thread.h	View		1 chunk	+0 lines, -3 lines	0 comments	Download
D	xfa/fwl/lightwidget/cfwl_caret.h	View		1 chunk	+0 lines, -28 lines	0 comments	Download
D	xfa/fwl/lightwidget/cfwl_caret.cpp	View		1 chunk	+0 lines, -55 lines	0 comments	Download
M	xfa/fwl/theme/cfwl_checkboxtp.cpp	View		2 chunks	+0 lines, -9 lines	0 comments	Download
M	xfa/fwl/theme/cfwl_comboboxtp.cpp	View		2 chunks	+1 line, -42 lines	0 comments	Download
M	xfa/fwl/theme/cfwl_datetimepickedtp.cpp	View		2 chunks	+2 lines, -41 lines	0 comments	Download
M	xfa/fwl/theme/cfwl_monthcalendartp.cpp	View		1 chunk	+0 lines, -8 lines	0 comments	Download
M	xfa/fwl/theme/cfwl_scrollbartp.cpp	View		2 chunks	+2 lines, -70 lines	0 comments	Download
M	xfa/fwl/theme/cfwl_utils.h	View		2 chunks	+2 lines, -24 lines	0 comments	Download
M	xfa/fwl/theme/cfwl_widgettp.h	View		2 chunks	+0 lines, -2 lines	0 comments	Download
M	xfa/fwl/theme/cfwl_widgettp.cpp	View		4 chunks	+3 lines, -47 lines	0 comments	Download

Messages

Total messages: 20 (7 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Tom Sepez

https://codereview.chromium.org/1874963002/diff/1/xfa/fwl/basewidget/ifwl_checkbox.h File xfa/fwl/basewidget/ifwl_checkbox.h (right): https://codereview.chromium.org/1874963002/diff/1/xfa/fwl/basewidget/ifwl_checkbox.h#newcode62 xfa/fwl/basewidget/ifwl_checkbox.h:62: #define FWL_EVTHASH_CKB_CheckStateChanged 2503252963 nit: I think the #define above ...

4 years, 8 months ago (2016-04-11 17:29:44 UTC) #3

dsinclair

4 years, 8 months ago (2016-04-11 19:47:33 UTC) #4

dsinclair

On 2016/04/11 19:47:33, dsinclair wrote: > https://codereview.chromium.org/1874963002/diff/1/xfa/fwl/basewidget/ifwl_checkbox.h > File xfa/fwl/basewidget/ifwl_checkbox.h (right): > > https://codereview.chromium.org/1874963002/diff/1/xfa/fwl/basewidget/ifwl_checkbox.h#newcode62 > ...

4 years, 8 months ago (2016-04-12 13:02:46 UTC) #5

Tom Sepez

On 2016/04/12 13:02:46, dsinclair wrote: > On 2016/04/11 19:47:33, dsinclair wrote: > > > https://codereview.chromium.org/1874963002/diff/1/xfa/fwl/basewidget/ifwl_checkbox.h ...

4 years, 8 months ago (2016-04-12 16:44:11 UTC) #6

dsinclair

On 2016/04/12 16:44:11, Tom Sepez wrote: > On 2016/04/12 13:02:46, dsinclair wrote: > > On ...

4 years, 8 months ago (2016-04-12 17:43:17 UTC) #7

On 2016/04/12 16:44:11, Tom Sepez wrote:
> On 2016/04/12 13:02:46, dsinclair wrote:
> > On 2016/04/11 19:47:33, dsinclair wrote:
> > >
> >
>
https://codereview.chromium.org/1874963002/diff/1/xfa/fwl/basewidget/ifwl_che...
> > > File xfa/fwl/basewidget/ifwl_checkbox.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1874963002/diff/1/xfa/fwl/basewidget/ifwl_che...
> > > xfa/fwl/basewidget/ifwl_checkbox.h:62: #define
> > FWL_EVTHASH_CKB_CheckStateChanged
> > > 2503252963
> > > On 2016/04/11 17:29:44, Tom Sepez wrote:
> > > > nit: I think the #define above indicates what string hashed to this
value.
> 
> > > > Maybe add a comment that says this.
> > > > 
> > > > In general, they act as if they have a collision-free hash function,
which
> > may
> > > > be true for a small set of keywords, but we should really centralize and
> > > assert
> > > > it someday.
> > > 
> > > 
> > > I don't think that's right in this case. FWL_EVTHASH_CKB_CheckStateChanged
> is
> > > passed into BEGIN_FWL_EVENT_DEF as the second argument which is the
> > > 'eventhashcode'. This code is never set into the event, it's only returned
> > > through GetClassID.
> > > 
> > > I, think, this is just a unique identifier for that type of event.
> > > 
> > >
> >
>
https://codereview.chromium.org/1874963002/diff/1/xfa/fwl/basewidget/ifwl_com...
> > > File xfa/fwl/basewidget/ifwl_combobox.h (left):
> > > 
> > >
> >
>
https://codereview.chromium.org/1874963002/diff/1/xfa/fwl/basewidget/ifwl_com...
> > > xfa/fwl/basewidget/ifwl_combobox.h:53: #define FWL_EVT_CMB_DropDown
> > > L"FWL_EVENT_CMB_PreDropDown"
> > > On 2016/04/11 17:29:44, Tom Sepez wrote:
> > > > same here.
> > > 
> > > Acknowledged.
> > 
> > 
> > Ping.
> Yes, but how they came up with that identifier is by (manually) hashing the
> above string.  If I were to pick the above string for use in some other
purpose,
> then I'd get a collision if I hashed it.  Which is way I want to keep the
string
> in the code in some manner -- otherwise, there's no telling how you got that
> number,

Filed pdfium:474 and added TODO's to each of the places we define an EVTHASH.