Cleanup FWL default values part II.

Cleanup FWL default values part II.

This CL cleans up the remaining default values in the fwl/core directory. Some
methods are renamed to better represent what they do.

Committed: https://pdfium.googlesource.com/pdfium/+/442997cbb3a9e970946dd294acd1498e9fa9bfc7

[dsinclair, 2016-12-07 16:34:04]

The CQ bit was checked by dsinclair@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 16:34:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dsinclair, 2016-12-07 16:43:17]

dsinclair@chromium.org changed reviewers:
+ tsepez@chromium.org, weili@chromium.org

[dsinclair, 2016-12-07 16:43:18]

PTAL.

[commit-bot: I haz the power, 2016-12-07 16:47:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-07 16:47:56]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-12-07 17:33:33]

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit.cpp
File xfa/fwl/core/cfwl_edit.cpp (right):

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:348: nLength = m_EdtEngine.GetSelRange(i, nStart);
nStart used uninitiaized?  Or is it an out parameter?  If the later, pass by
&nStart instead?

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:349: wsTemp = m_EdtEngine.GetText(nStart, nLength);
wsTemp local to this block?  Or eliminate

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:351: wsTemp.clear();
no need to clear on each iteration if local.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:446: void
CFWL_Edit::On_TextChanged(FDE_TXTEDT_TEXTCHANGE_INFO& ChangeInfo) {
nit: pass by const reference?

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:490: bool CFWL_Edit::On_Validate(CFX_WideString&
wsText) {
nit: pass by const reference?

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:990: int32_t
CFWL_Edit::AddDoRecord(IFDE_TxtEdtDoRecord* pRecord) {
This should be called with the unique_ptr<> itself rather than making one at
line 1002 (while we're at it)  ... and propagate upwards until we can replace a
|new| with MakeUnique<>.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit.h
File xfa/fwl/core/cfwl_edit.h (right):

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.h:104: void On_CaretChanged();
nit: can we lose the _ in these names?

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_spinb...
File xfa/fwl/core/cfwl_spinbutton.cpp (right):

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_spinb...
xfa/fwl/core/cfwl_spinbutton.cpp:295: if (!IsButtonEnabled(false))
This just reads strangely ... sounds like a no-op?

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_widge...
File xfa/fwl/core/cfwl_widgetmgr.h (right):

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_widge...
xfa/fwl/core/cfwl_widgetmgr.h:100: void SetWidgetIndex(CFWL_Widget* pWidget);
this may need a new name if index is always 0.

[dsinclair, 2016-12-07 18:28:49]

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit.cpp
File xfa/fwl/core/cfwl_edit.cpp (right):

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:348: nLength = m_EdtEngine.GetSelRange(i, nStart);
On 2016/12/07 17:33:32, Tom Sepez wrote:
> nStart used uninitiaized?  Or is it an out parameter?  If the later, pass by
> &nStart instead?

Done. It was being passed as an int32_t&, changed it to int32_t*.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:349: wsTemp = m_EdtEngine.GetText(nStart, nLength);
On 2016/12/07 17:33:32, Tom Sepez wrote:
> wsTemp local to this block?  Or eliminate

Done.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:351: wsTemp.clear();
On 2016/12/07 17:33:32, Tom Sepez wrote:
> no need to clear on each iteration if local.

Done.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:446: void
CFWL_Edit::On_TextChanged(FDE_TXTEDT_TEXTCHANGE_INFO& ChangeInfo) {
On 2016/12/07 17:33:32, Tom Sepez wrote:
> nit: pass by const reference?

Done.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:490: bool CFWL_Edit::On_Validate(CFX_WideString&
wsText) {
On 2016/12/07 17:33:32, Tom Sepez wrote:
> nit: pass by const reference?

Done.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:990: int32_t
CFWL_Edit::AddDoRecord(IFDE_TxtEdtDoRecord* pRecord) {
On 2016/12/07 17:33:32, Tom Sepez wrote:
> This should be called with the unique_ptr<> itself rather than making one at
> line 1002 (while we're at it)  ... and propagate upwards until we can replace
a
> |new| with MakeUnique<>.

Done.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit.h
File xfa/fwl/core/cfwl_edit.h (right):

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.h:104: void On_CaretChanged();
On 2016/12/07 17:33:32, Tom Sepez wrote:
> nit: can we lose the _ in these names?

Done.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_spinb...
File xfa/fwl/core/cfwl_spinbutton.cpp (right):

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_spinb...
xfa/fwl/core/cfwl_spinbutton.cpp:295: if (!IsButtonEnabled(false))
On 2016/12/07 17:33:32, Tom Sepez wrote:
> This just reads strangely ... sounds like a no-op?

Done.

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_widge...
File xfa/fwl/core/cfwl_widgetmgr.h (right):

https://codereview.chromium.org/2557103002/diff/20001/xfa/fwl/core/cfwl_widge...
xfa/fwl/core/cfwl_widgetmgr.h:100: void SetWidgetIndex(CFWL_Widget* pWidget);
On 2016/12/07 17:33:33, Tom Sepez wrote:
> this may need a new name if index is always 0.

Done.

[dsinclair, 2016-12-07 18:28:53]

The CQ bit was checked by dsinclair@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 18:28:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-07 18:43:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-07 18:43:27]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-12-07 19:43:49]

lgtm

[dsinclair, 2016-12-08 01:23:48]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-12-08 01:23:48]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2557103002/#ps60001
(title: "Rebase to master")

[commit-bot: I haz the power, 2016-12-08 01:23:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 01:44:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-08 01:44:21]

Failed to apply patch for xfa/fde/cfde_txtedtengine.h:
While running git apply --index -p1;
  error: patch failed: xfa/fde/cfde_txtedtengine.h:31
  error: xfa/fde/cfde_txtedtengine.h: patch does not apply

Patch:       xfa/fde/cfde_txtedtengine.h
Index: xfa/fde/cfde_txtedtengine.h
diff --git a/xfa/fde/cfde_txtedtengine.h b/xfa/fde/cfde_txtedtengine.h
index
bd02c9b9e77c068e343a04a91279e703af215e05..9e0c3920d5b9f9d82b41301cbce6cc565b5b79b5
100644
--- a/xfa/fde/cfde_txtedtengine.h
+++ b/xfa/fde/cfde_txtedtengine.h
@@ -31,7 +31,7 @@ class CFDE_TxtEdtEngine {
   void SetTextByStream(IFGAS_Stream* pStream);
   void SetText(const CFX_WideString& wsText);
   int32_t GetTextLength() const;
-  CFX_WideString GetText(int32_t nStart, int32_t nCount = -1) const;
+  CFX_WideString GetText(int32_t nStart, int32_t nCount) const;
   void ClearText();
 
   int32_t GetCaretRect(CFX_RectF& rtCaret) const;
@@ -54,11 +54,11 @@ class CFDE_TxtEdtEngine {
   void SetLimit(int32_t nLimit);
   void SetAliasChar(FX_WCHAR wcAlias);
 
-  void RemoveSelRange(int32_t nStart, int32_t nCount = -1);
+  void RemoveSelRange(int32_t nStart, int32_t nCount);
 
-  void AddSelRange(int32_t nStart, int32_t nCount = -1);
+  void AddSelRange(int32_t nStart, int32_t nCount);
   int32_t CountSelRanges() const;
-  int32_t GetSelRange(int32_t nIndex, int32_t& nStart) const;
+  int32_t GetSelRange(int32_t nIndex, int32_t* nStart) const;
   void ClearSelection();
 
   bool Redo(const IFDE_TxtEdtDoRecord* pRecord);

[dsinclair, 2016-12-08 01:48:21]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-12-08 01:48:21]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2557103002/#ps80001
(title: "Rebase to master")

[commit-bot: I haz the power, 2016-12-08 01:48:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 01:58:41]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481161701054400,
"parent_rev": "31559c91c4983b42361415d30b0b2a518c7ef383", "commit_rev":
"442997cbb3a9e970946dd294acd1498e9fa9bfc7"}

[commit-bot: I haz the power, 2016-12-08 01:58:46]

Description was changed from

==========
Cleanup FWL default values part II.

This CL cleans up the remaining default values in the fwl/core directory. Some
methods are renamed to better represent what they do.
==========

to

==========
Cleanup FWL default values part II.

This CL cleans up the remaining default values in the fwl/core directory. Some
methods are renamed to better represent what they do.

Committed:
https://pdfium.googlesource.com/pdfium/+/442997cbb3a9e970946dd294acd1498e9fa9...
==========

[commit-bot: I haz the power, 2016-12-08 01:58:47]

Committed patchset #5 (id:80001) as
https://pdfium.googlesource.com/pdfium/+/442997cbb3a9e970946dd294acd1498e9fa9...