Reland 'Use FrameSelection::selectedText where possible.'

Reland https://codereview.chromium.org/1377963004/

Use FrameSelection::selectedText where possible.

Substitute occurrences of selection -> range -> (text or html) conversions with
direct selection -> (text or html) conversions using existing functions.

BUG=

[kotenkov, 2015-10-23 15:39:45]

Description was changed from

==========
Re-land https://codereview.chromium.org/1377963004/

Use FrameSelection::selectedText where possible.

Substitute occurrences of selection -> range -> (text or html) conversions with
direct selection -> (text or html) conversions using existing functions.

BUG=
==========

to

==========
Reland https://codereview.chromium.org/1377963004/

Use FrameSelection::selectedText where possible.

Substitute occurrences of selection -> range -> (text or html) conversions with
direct selection -> (text or html) conversions using existing functions.

BUG=
==========

[kotenkov, 2015-10-23 15:40:41]

kotenkov@yandex-team.ru changed reviewers:
+ yosin@chromium.org

[kotenkov, 2015-10-23 15:40:42]

I've introduced changes that you suggested here:
https://codereview.chromium.org/1377963004#msg40
I'm compiling a release build to run perfomance tests, I'll report results
later.
Would you please run try jobs for me?
I have a bad feeling about layout tests =)

[kotenkov, 2015-10-23 17:44:53]

> I'm compiling a release build to run perfomance tests, I'll report results
> later.

I've run speedometer perftest locally and I don't see any regressions (I can see
them in patchset #1). Is there a way to run perftests on bots while the CL is
being reviewed?

[yosin_UTC9, 2015-10-26 13:59:56]

On 2015/10/23 at 17:44:53, kotenkov wrote:
> > I'm compiling a release build to run perfomance tests, I'll report results
> > later.
> 
> I've run speedometer perftest locally and I don't see any regressions (I can
see them in patchset #1). Is there a way to run perftests on bots while the CL
is being reviewed?

I don't know. Local run should be enough.

Sorry for late response.
I'm attending TPAC 2015. I access review tool limited time.
I'll be back to office on Oct 30.

[yosin_UTC9, 2015-10-26 14:01:21]

On 2015/10/23 at 15:40:42, kotenkov wrote:
> I've introduced changes that you suggested here:
https://codereview.chromium.org/1377963004#msg40
> I'm compiling a release build to run perfomance tests, I'll report results
later.
> Would you please run try jobs for me?
> I have a bad feeling about layout tests =)

trybots are running.

[yosin_UTC9, 2015-10-26 14:01:31]

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

[commit-bot: I haz the power, 2015-10-26 14:02:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412713003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412713003/20001

[commit-bot: I haz the power, 2015-10-26 14:22:02]

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

[commit-bot: I haz the power, 2015-10-26 14:22:02]

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

[yosin_UTC9, 2015-10-27 01:07:52]

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

[commit-bot: I haz the power, 2015-10-27 01:08:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412713003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412713003/20001

[yosin_UTC9, 2015-10-27 01:11:20]

I believe bot failures are caused by bot flakinees, since
{linux,mac,win}_blink_rel are succeeded.

ktenkov@, could you double check?

[commit-bot: I haz the power, 2015-10-27 01:28:04]

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

[commit-bot: I haz the power, 2015-10-27 01:28:04]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[kotenkov, 2015-10-27 14:06:31]

On 2015/10/27 01:11:20, Yosi_OOO_til_Oct_30 wrote:
> I believe bot failures are caused by bot flakinees, since
> {linux,mac,win}_blink_rel are succeeded.
> 
> ktenkov@, could you double check?

I checked, my changes cause the failure, I'll investigate further.

[kotenkov, 2015-10-27 16:55:33]

Indeed it was my fault. I've forgotten to let observers know about changed
selection.
To fix it I used |FrameSelection::setSelection()| before setting the selection
without validation, so that all observers are informed.
PTAL and please run try jobs for me.

[yosin_UTC9, 2015-10-28 04:33:17]

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

[commit-bot: I haz the power, 2015-10-28 04:33:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412713003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412713003/40001

[commit-bot: I haz the power, 2015-10-28 05:35:50]

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

[commit-bot: I haz the power, 2015-10-28 05:35:51]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[kotenkov, 2015-10-28 07:32:12]

On 2015/10/28 05:35:51, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

It seems to me that failed test is not related to my changes. I think the test
doesn't event create a renderer.
It passes on my local linux and mac. I'm getting a checkout on windows to check
it there, but it may take a while.

[kotenkov, 2015-10-28 07:36:24]

On 2015/10/28 07:32:12, kotenkov wrote:
> On 2015/10/28 05:35:51, commit-bot: I haz the power wrote:
> > Dry run: Try jobs failed on following builders:
> >   win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
> 
> It seems to me that failed test is not related to my changes. I think the test
> doesn't event create a renderer.
> It passes on my local linux and mac. I'm getting a checkout on windows to
check
> it there, but it may take a while.

Hmm, there is also fast/forms/textfield-lastchange-was-useredit.html and it is
surely related, I'll investigate it first.

[kotenkov, 2015-10-28 08:27:26]

On 2015/10/28 07:36:24, kotenkov wrote:
> On 2015/10/28 07:32:12, kotenkov wrote:
> > On 2015/10/28 05:35:51, commit-bot: I haz the power wrote:
> > > Dry run: Try jobs failed on following builders:
> > >   win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
> > 
> > It seems to me that failed test is not related to my changes. I think the
test
> > doesn't event create a renderer.
> > It passes on my local linux and mac. I'm getting a checkout on windows to
> check
> > it there, but it may take a while.
> 
> Hmm, there is also fast/forms/textfield-lastchange-was-useredit.html and it is
> surely related, I'll investigate it first.

The test crashed due to ASSERTION FAILED: !node.needsDistributionRecalc() while
comparing positions. I used an existing function in SelectionEditor (which I
should have used in the first place).
Would you please run try jobs for me?

[yosin_UTC9, 2015-10-28 22:55:22]

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

[commit-bot: I haz the power, 2015-10-28 22:56:05]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412713003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412713003/60001

[commit-bot: I haz the power, 2015-10-29 00:18:04]

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

[commit-bot: I haz the power, 2015-10-29 00:18:04]

Dry run: This issue passed the CQ dry run.

[kotenkov, 2015-10-29 06:05:26]

kotenkov@yandex-team.ru changed reviewers:
+ tkent@chromium.org

[kotenkov, 2015-10-29 06:05:27]

Adding tkent as an owner. I have fixed the regression, PTAL

[yosin_UTC9, 2015-10-29 06:44:48]

https://codereview.chromium.org/1412713003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/1412713003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:862:
setSelection(selection, options);
FrameSelection::setSelection() calls SelectionEditor::setSelection(). So, we
update
VisibleSelection{InDOMTree,InComposedTree} twice, one SE:setSelection(), another
is
SE::setSelectionInTextFormControl().

What do you think about introducing SetSelectionOptions::ForTextFormControl and
makes FrameSelection::setSelection() to call SE:setSelection() or
SE:setSelectionInTextForm()

https://codereview.chromium.org/1412713003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/1412713003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:862:
setSelection(selection, options);
FrameSelection::setSelection() and SelectionEditor::setWithoutValidation() calls
SelectionEdtior::adjustSelectionInComposedTreeTwice(). It waste time.
Can we avoid this?

One idea is
Introduce SetSelectionOptions::ForTextFormControl then

void FrameSelection::setSelection(...) 
{
  ...
  if (options & ForTextFormControl)
    m_selectionEditor->setSelectionInTextFormControl(selection);
  else
    m_selectionEditor->setSelection(selection);
}

void SelectionEditor::setSelectionInTextFormControl(const VisibleSelection&
selection)
{
  // Since, we know |selection| doesn't cross shadow boundaries
  m_selection = selection;
 
m_selectionInComposedTree.setWithoutValidation(toPositionInComposedTree(selection.base()),
toPositionInComposedTree(selection.extent());
}

I'm on the way to home from Sapporo. Just quick response.

[yosin_UTC9, 2015-10-29 06:50:51]

How about split this patch into two? One for TextFormControl changes and another
is for others.
It seems changes for TextFormControl causes trouble.