Convert GetWidgetRect to return rect.

Convert GetWidgetRect to return rect.

Current GetWidgetRect accepts the rect as an out-param. This CL converts the
code to return the rect instead.

Committed: https://pdfium.googlesource.com/pdfium/+/da911bc12558667555266425d0b7e83296e8cc7d

[dsinclair, 2016-12-07 21:09:10]

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

[dsinclair, 2016-12-07 21:09:11]

PTAL.

This one's a bit tricky in places so make sure my logic is sound ....

https://codereview.chromium.org/2556873004/diff/1/xfa/fwl/core/cfwl_widget.cpp
File xfa/fwl/core/cfwl_widget.cpp (left):

https://codereview.chromium.org/2556873004/diff/1/xfa/fwl/core/cfwl_widget.cp...
xfa/fwl/core/cfwl_widget.cpp:237: pWidget->GetWidgetRect(r, false);
If pWidget isn't a subclass that overrides GetWidgetRect this will be different
....

https://codereview.chromium.org/2556873004/diff/1/xfa/fwl/core/cfwl_widget.cp...
xfa/fwl/core/cfwl_widget.cpp:264: parent->GetWidgetRect(rect, false);
potential difference if parent is a subclass that doesn't override
GetWidgetRect...

[dsinclair, 2016-12-07 21:18:20]

On 2016/12/07 21:09:11, dsinclair wrote:
> PTAL.
> 
> This one's a bit tricky in places so make sure my logic is sound ....
> 
> https://codereview.chromium.org/2556873004/diff/1/xfa/fwl/core/cfwl_widget.cpp
> File xfa/fwl/core/cfwl_widget.cpp (left):
> 
>
https://codereview.chromium.org/2556873004/diff/1/xfa/fwl/core/cfwl_widget.cp...
> xfa/fwl/core/cfwl_widget.cpp:237: pWidget->GetWidgetRect(r, false);
> If pWidget isn't a subclass that overrides GetWidgetRect this will be
different
> ....
> 
>
https://codereview.chromium.org/2556873004/diff/1/xfa/fwl/core/cfwl_widget.cp...
> xfa/fwl/core/cfwl_widget.cpp:264: parent->GetWidgetRect(rect, false);
> potential difference if parent is a subclass that doesn't override
> GetWidgetRect...


Hold off on this for now, I think I can simplify some things ....

[dsinclair, 2016-12-07 21:43:43]

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

[commit-bot: I haz the power, 2016-12-07 21:43:44]

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 21:44:31]

PTAL.

There are only about 5 places where GetWidgetRect is called with the bAutoSize
parameter set to true. This patchset splits the GetWidgetRect into two calls
depending on the flag and in the 2 clases where it's needed implements the
AutoSize variant.

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_combo...
File xfa/fwl/core/cfwl_combobox.cpp (right):

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_combo...
xfa/fwl/core/cfwl_combobox.cpp:347: m_rtList =
m_pListBox->GetAutosizedWidgetRect();
True here.

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_datet...
File xfa/fwl/core/cfwl_datetimepicker.cpp (left):

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_datet...
xfa/fwl/core/cfwl_datetimepicker.cpp:50: m_pMonthCal->GetWidgetRect(rtMonthCal,
true);
true here.

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_datet...
xfa/fwl/core/cfwl_datetimepicker.cpp:116: m_pMonthCal->GetWidgetRect(rtMonthCal,
true);
true here.

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_datet...
xfa/fwl/core/cfwl_datetimepicker.cpp:387: m_pMonthCal->GetWidgetRect(rtMonthCal,
true);
true.

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_datet...
xfa/fwl/core/cfwl_datetimepicker.cpp:466: m_pMonthCal->GetWidgetRect(rtMonthCal,
true);
true

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_widge...
File xfa/fwl/core/cfwl_widget.cpp (right):

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_widge...
xfa/fwl/core/cfwl_widget.cpp:69: return CFX_RectF();
This should never be called, so I just went with the empty implementation.

[Tom Sepez, 2016-12-07 21:53:20]

If you're really ambitious someday, it's always struck me that the "calculus of
rectangles" gets easier if rects are immutable objects, and the various
operations just return the result rather than modifying the arg.  Rects are
small enough, and with RVO it gets even better.

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

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:113: int32_t iTextLen = m_EdtEngine.GetTextLength();
nit: local needed?

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:152: if
(m_pVertScrollBar->GetWidgetRect().Contains(fx, fy))
nice.

https://codereview.chromium.org/2556873004/diff/40001/xfa/fxfa/app/xfa_fffiel...
File xfa/fxfa/app/xfa_fffield.cpp (right):

https://codereview.chromium.org/2556873004/diff/40001/xfa/fxfa/app/xfa_fffiel...
xfa/fxfa/app/xfa_fffield.cpp:595: return m_pNormalWidget ?
m_pNormalWidget->GetWidgetRect().Contains(fx, fy)
nit: return pNormalWidget && ...

[commit-bot: I haz the power, 2016-12-07 21:58:54]

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

[commit-bot: I haz the power, 2016-12-07 21:58:55]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-12-07 22:09:37]

lgtm

Otherwise OK.

[dsinclair, 2016-12-08 02:35:06]

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

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:113: int32_t iTextLen = m_EdtEngine.GetTextLength();
On 2016/12/07 21:53:20, Tom Sepez wrote:
> nit: local needed?

Done.

https://codereview.chromium.org/2556873004/diff/40001/xfa/fwl/core/cfwl_edit....
xfa/fwl/core/cfwl_edit.cpp:152: if
(m_pVertScrollBar->GetWidgetRect().Contains(fx, fy))
On 2016/12/07 21:53:20, Tom Sepez wrote:
> nice.

Acknowledged.

https://codereview.chromium.org/2556873004/diff/40001/xfa/fxfa/app/xfa_fffiel...
File xfa/fxfa/app/xfa_fffield.cpp (right):

https://codereview.chromium.org/2556873004/diff/40001/xfa/fxfa/app/xfa_fffiel...
xfa/fxfa/app/xfa_fffield.cpp:595: return m_pNormalWidget ?
m_pNormalWidget->GetWidgetRect().Contains(fx, fy)
On 2016/12/07 21:53:20, Tom Sepez wrote:
> nit: return pNormalWidget && ...

Done.

[dsinclair, 2016-12-08 02:35:59]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-12-08 02:35:59]

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/2556873004/#ps60001
(title: "Review feedback")

[commit-bot: I haz the power, 2016-12-08 02:36:02]

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 02:47:00]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481164559090360,
"parent_rev": "67c6ca3cd86b6a31c888264bb8067c6c4324e56c", "commit_rev":
"da911bc12558667555266425d0b7e83296e8cc7d"}

[commit-bot: I haz the power, 2016-12-08 02:47:03]

Description was changed from

==========
Convert GetWidgetRect to return rect.

Current GetWidgetRect accepts the rect as an out-param. This CL converts the
code to return the rect instead.
==========

to

==========
Convert GetWidgetRect to return rect.

Current GetWidgetRect accepts the rect as an out-param. This CL converts the
code to return the rect instead.

Committed:
https://pdfium.googlesource.com/pdfium/+/da911bc12558667555266425d0b7e83296e8...
==========

[commit-bot: I haz the power, 2016-12-08 02:47:04]

Committed patchset #4 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/da911bc12558667555266425d0b7e83296e8...