Rename *Focus*-methods of FrameSelection to clarify its public API

Rename *Focus*-methods of FrameSelection to clarify its public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2876973003
Cr-Commit-Position: refs/heads/master@{#472129}
Committed: https://chromium.googlesource.com/chromium/src/+/582a4088398728aa9b1ab672dd92ecc9b9d589db

[hugoh_UTC2, 2017-05-12 13:30:31]

Description was changed from

==========
Rename *Focus*-methods of FrameSelection to clarify public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889 - follow up on [1]
TEST=No behavior change

[1] = crrev.com/2841093002
==========

to

==========
Rename *Focus*-methods of FrameSelection to clarify public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889 - follow up on [1]
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[hugoh_UTC2, 2017-05-12 13:59:57]

Description was changed from

==========
Rename *Focus*-methods of FrameSelection to clarify public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889 - follow up on [1]
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Rename *Focus*-methods of FrameSelection to clarify public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889 - follow up on [1]
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[hugoh_UTC2, 2017-05-12 14:00:25]

Description was changed from

==========
Rename *Focus*-methods of FrameSelection to clarify public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889 - follow up on [1]
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Rename *Focus*-methods of FrameSelection to clarify its public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889 - follow up on [1]
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[hugoh_UTC2, 2017-05-12 14:01:01]

xiaocheng@, since we talked about this earlier [1], does this look OK to you?

BTW, when looking at these renames I wonder why these 3 *FrameIsFocused* methods
(SetFrameIsFocused, FrameIsFocused, FrameIsFocusedAndActive) aren't in
LocalFrame.cpp ... They don't tell anything about the state of the selection (?)
- they only report the state of the frame, right?

[1] https://codereview.chromium.org/2841093002/#msg52

[hugoh_UTC2, 2017-05-12 14:01:50]

hugoh@opera.com changed reviewers:
+ xiaochengh@chromium.org

[Xiaocheng, 2017-05-12 18:18:29]

xiaochengh@chromium.org changed reviewers:
+ tkent@chromium.org, yosin@chromium.org

[Xiaocheng, 2017-05-12 18:18:30]

+yosin, tkent

I agree that it seems better to maintain the frame's focused state in LocalFrame
instead of FrameSelection. So we should move the following to LocalFrame:
- FS::SetFocused
- FS::IsFocused
- FS::IsFocusedAndActive
- FS::PageActivationChanged

FS::FocusedOrActiveStateChanged is questionable because it does have something
to do with caret, selection and text input.

WDYT?

[yosin_UTC9, 2017-05-15 02:11:54]

On 2017/05/12 at 18:18:30, xiaochengh wrote:
> +yosin, tkent
> 
> I agree that it seems better to maintain the frame's focused state in
LocalFrame instead of FrameSelection. So we should move the following to
LocalFrame:
> - FS::SetFocused
> - FS::IsFocused
> - FS::IsFocusedAndActive
> - FS::PageActivationChanged
> 
> FS::FocusedOrActiveStateChanged is questionable because it does have something
to do with caret, selection and text input.
> 
> WDYT?

lgtm for rename

Agree on xiaochengh@ to move FS::focused_ to LocalFrame.
To avoid regression on moving FS::focused_ to LocalFrame, we should land this
rename patch.

Because FS::SetFocused() has call guard of |FocusedOrActiveStateChanged()| which
calls
|Document::UpdateStyleAndLayoutTree()|, Should be PendingIgnoreStyleSheet?, 
we may update layout more than |FS::focused_|, just guess.

void FrameSelection::SetFocused(bool flag) {
  if (focused_ == flag)
    return;
  focused_ = flag;

  FocusedOrActiveStateChanged();
}

Note: |FS::focused_| is initialized as
      focused_(frame.GetPage() &&
               frame.GetPage()->GetFocusController().FocusedFrame() == frame


It seems moving FS::IsFocusedAndActive() to LocalFrame in parallel with moving
|FS::focused_|,
since |FS::focused_| is local copy of |FocusedController().FocusedFrame() ==
this_local_frame|.

[hugoh_UTC2, 2017-05-15 08:42:09]

hugoh@opera.com changed reviewers:
+ dmazzoni@chromium.org

[hugoh_UTC2, 2017-05-15 08:45:43]

OK, let's start with this rename then.

tkent@, could you approve /web and /core ?

dmazzoni@, could you approve /modules/accessibility ?

[hugoh_UTC2, 2017-05-15 11:35:22]

Description was changed from

==========
Rename *Focus*-methods of FrameSelection to clarify its public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889 - follow up on [1]
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Rename *Focus*-methods of FrameSelection to clarify its public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[dmazzoni, 2017-05-15 20:42:55]

lgtm for accessibility

[tkent, 2017-05-15 21:16:53]

lgtm

[yosin_UTC9, 2017-05-16 01:13:23]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2017-05-16 01:14:58]

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, 2017-05-16 01:48:01]

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

[commit-bot: I haz the power, 2017-05-16 01:48:02]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[hugoh_UTC2, 2017-05-16 08:40:04]

The CQ bit was checked by hugoh@opera.com

[commit-bot: I haz the power, 2017-05-16 08:40:27]

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, 2017-05-16 10:06:47]

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

[commit-bot: I haz the power, 2017-05-16 10:06:49]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hugoh_UTC2, 2017-05-16 13:07:27]

The CQ bit was checked by hugoh@opera.com

[commit-bot: I haz the power, 2017-05-16 13:07:53]

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, 2017-05-16 14:37:19]

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

[commit-bot: I haz the power, 2017-05-16 14:37:20]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hugoh_UTC2, 2017-05-16 14:53:13]

The CQ bit was checked by hugoh@opera.com

[commit-bot: I haz the power, 2017-05-16 14:53:33]

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, 2017-05-16 16:36:03]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1494946393083620, "parent_rev":
"468a6511775fca06ef0ae9d483cd2dc5668fdff8", "commit_rev":
"582a4088398728aa9b1ab672dd92ecc9b9d589db"}

[commit-bot: I haz the power, 2017-05-16 16:36:14]

Description was changed from

==========
Rename *Focus*-methods of FrameSelection to clarify its public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Rename *Focus*-methods of FrameSelection to clarify its public API

As we've now added FS::SelectionHasFocus in [1], let's rename
FS::*IsFocused* to FS::*FrameIsFocused* to emphasize that these
methods give information about the Frame (in contrast to
FS::SelectionHasFocus).

BUG=715889
TEST=No behavior change

[1] = crrev.com/2841093002
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2876973003
Cr-Commit-Position: refs/heads/master@{#472129}
Committed:
https://chromium.googlesource.com/chromium/src/+/582a4088398728aa9b1ab672dd92...
==========

[commit-bot: I haz the power, 2017-05-16 16:36:16]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/582a4088398728aa9b1ab672dd92...