Fix long press touch selection on empty text fields

Fix long press touch selection on empty text fields

Desired behavior when long press happens on an empty text field is to
activate touch selection and show the quick menu if there is anything
available to show in the quick menu. Otherwise, the regular context menu
should be shown. This was regressed due to changes in context menu
source type that added LONG_PRESS and LONG_TAP types. In addition to
fixing this regression, this CL fixes the regression on the new tab page
that shows an incorrectly placed quick menu instead of the context menu.

BUG=594101
Committed: https://crrev.com/41355a89adc05a40f8d6eb1e60a600edb02f8de1
Cr-Commit-Position: refs/heads/master@{#407059}

[mohsen, 2016-07-06 16:50:50]

mohsen@chromium.org changed reviewers:
+ tdresser@chromium.org

[mohsen, 2016-07-06 16:50:50]

Can you take a look?

I think a test is necessary here to prevent future regressions. I tried to write
some tests, but I was unsuccessful. This code path needs
WebContentsViewAura::ShowContextMenu() to be called, but in browser tests,
context menu requests are intercepted by content::Shell which sets itself as a
delegate for WebContents. WDYT?

[tdresser, 2016-07-06 17:35:32]

On 2016/07/06 16:50:50, mohsen wrote:
> Can you take a look?
> 
> I think a test is necessary here to prevent future regressions. I tried to
write
> some tests, but I was unsuccessful. This code path needs
> WebContentsViewAura::ShowContextMenu() to be called, but in browser tests,
> context menu requests are intercepted by content::Shell which sets itself as a
> delegate for WebContents. WDYT?

Hmmm, it isn't particularly clear to me how this should work.

Could you wire things up correctly in web_contents_view_aura_unittest?

Worst case, we might be able to use a layout test here - what we want is an
integration test...

[mohsen, 2016-07-13 21:19:26]

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

[commit-bot: I haz the power, 2016-07-13 21:20:45]

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-07-13 22:25:33]

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

[commit-bot: I haz the power, 2016-07-13 22:25:34]

Dry run: This issue passed the CQ dry run.

[mohsen, 2016-07-14 01:43:29]

For tests, I ended up changing how content shell, and as a result, browser tests
handle context menu (https://crrev.com/2132983002, not yet landed) and added a
test here. Can you take a look at the CL now?

[mohsen, 2016-07-14 01:43:30]

For tests, I ended up changing how content shell, and as a result, browser tests
handle context menu (https://crrev.com/2132983002, not yet landed) and added a
test here. Can you take a look at the CL now?

[tdresser, 2016-07-14 12:13:40]

LGTM!

https://codereview.chromium.org/2124913002/diff/20001/content/browser/rendere...
File
content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc
(right):

https://codereview.chromium.org/2124913002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc:323:
ASSERT_TRUE(EmptyTextfield());
It isn't immediately clear to me whether this is just asserting that the
textfield is empty, or actually emptying it.

[mohsen, 2016-07-21 22:00:32]

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

[commit-bot: I haz the power, 2016-07-21 22:01:03]

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-07-22 00:52:22]

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

[commit-bot: I haz the power, 2016-07-22 00:52:24]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[mohsen, 2016-07-22 01:22:47]

Still waiting for https://crrev.com/2132983002 to land...

https://codereview.chromium.org/2124913002/diff/20001/content/browser/rendere...
File
content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc
(right):

https://codereview.chromium.org/2124913002/diff/20001/content/browser/rendere...
content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc:323:
ASSERT_TRUE(EmptyTextfield());
On 2016/07/14 at 12:13:40, tdresser wrote:
> It isn't immediately clear to me whether this is just asserting that the
textfield is empty, or actually emptying it.

Renamed!

[mohsen, 2016-07-22 03:37:43]

The CQ bit was checked by mohsen@chromium.org

[mohsen, 2016-07-22 03:37:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/2124913002/#ps40001
(title: "EmptyTextfield -> ClearTextfield")

[commit-bot: I haz the power, 2016-07-22 03:38:08]

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-07-22 04:18:10]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-22 04:20:01]

Description was changed from

==========
Fix long press touch selection on empty text fields

Desired behavior when long press happens on an empty text field is to
activate touch selection and show the quick menu if there is anything
available to show in the quick menu. Otherwise, the regular context menu
should be shown. This was regressed due to changes in context menu
source type that added LONG_PRESS and LONG_TAP types. In addition to
fixing this regression, this CL fixes the regression on the new tab page
that shows an incorrectly placed quick menu instead of the context menu.

BUG=594101
==========

to

==========
Fix long press touch selection on empty text fields

Desired behavior when long press happens on an empty text field is to
activate touch selection and show the quick menu if there is anything
available to show in the quick menu. Otherwise, the regular context menu
should be shown. This was regressed due to changes in context menu
source type that added LONG_PRESS and LONG_TAP types. In addition to
fixing this regression, this CL fixes the regression on the new tab page
that shows an incorrectly placed quick menu instead of the context menu.

BUG=594101

Committed: https://crrev.com/41355a89adc05a40f8d6eb1e60a600edb02f8de1
Cr-Commit-Position: refs/heads/master@{#407059}
==========

[commit-bot: I haz the power, 2016-07-22 04:20:02]

Patchset 3 (id:??) landed as
https://crrev.com/41355a89adc05a40f8d6eb1e60a600edb02f8de1
Cr-Commit-Position: refs/heads/master@{#407059}