[TTS] Promote clicks on AMP carousel to a separate Tab.

[TTS] Promote clicks on AMP carousel to a separate Tab.

An AMP carousel is part of the SRP (Search Result Page) and clicking on one
just does sub-frame loading.  Almost everything works fine except for
issue 653760 which causes the subframe to scroll up but not down.

This change simply promotes clicks on the AMP carousel into a separate Tab.

BUG=645325
Committed: https://crrev.com/ffd75360db3fc16ec6b4cb28c5b68b534f8ac55d
Cr-Commit-Position: refs/heads/master@{#426607}

[Donn Denman, 2016-10-18 19:47:21]

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

[Donn Denman, 2016-10-18 19:47:42]

donnd@chromium.org changed reviewers:
+ twellington@chromium.org

[Donn Denman, 2016-10-18 19:47:43]

Theresa, PTAL.  Thanks!

[commit-bot: I haz the power, 2016-10-18 19:47:47]

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-10-18 19:50:39]

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

[commit-bot: I haz the power, 2016-10-18 19:50:39]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Donn Denman, 2016-10-18 19:56:50]

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

[commit-bot: I haz the power, 2016-10-18 19:57:21]

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-10-18 21:19:00]

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

[commit-bot: I haz the power, 2016-10-18 21:19:01]

Dry run: This issue passed the CQ dry run.

[Theresa, 2016-10-19 16:21:02]

https://codereview.chromium.org/2429913002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
(right):

https://codereview.chromium.org/2429913002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:916:
&& mSearchPanel.didTouchContent()) {
I think some comments here would be helpful. #didTouchContent() and
isExternalUrl don't seem to be amp specific -- are there other cases where we'll
end up promoting to a new tab with this new logic?

[Donn Denman, 2016-10-20 04:57:00]

Theresa, PTAL when convenient.

Thanks!

https://codereview.chromium.org/2429913002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
(right):

https://codereview.chromium.org/2429913002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:916:
&& mSearchPanel.didTouchContent()) {
On 2016/10/19 16:21:02, Theresa Wellington wrote:
> I think some comments here would be helpful. #didTouchContent() and
> isExternalUrl don't seem to be amp specific -- are there other cases where
we'll
> end up promoting to a new tab with this new logic?

Doh, forgot to check the URL! :-)

[Theresa, 2016-10-20 20:20:59]

lgtm

[Donn Denman, 2016-10-20 20:31:25]

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

[commit-bot: I haz the power, 2016-10-20 20:32:33]

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-10-20 21:18:17]

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

[commit-bot: I haz the power, 2016-10-20 21:18:17]

Dry run: This issue passed the CQ dry run.

[Donn Denman, 2016-10-20 21:32:45]

The CQ bit was checked by donnd@chromium.org

[commit-bot: I haz the power, 2016-10-20 21:33:14]

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-10-20 21:45:18]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-21 13:22:56]

Description was changed from

==========
[TTS] Promote clicks on AMP carousel to a separate Tab.

An AMP carousel is part of the SRP (Search Result Page) and clicking on one
just does sub-frame loading.  Almost everything works fine except for
issue 653760 which causes the subframe to scroll up but not down.

This change simply promotes clicks on the AMP carousel into a separate Tab.

BUG=645325
==========

to

==========
[TTS] Promote clicks on AMP carousel to a separate Tab.

An AMP carousel is part of the SRP (Search Result Page) and clicking on one
just does sub-frame loading.  Almost everything works fine except for
issue 653760 which causes the subframe to scroll up but not down.

This change simply promotes clicks on the AMP carousel into a separate Tab.

BUG=645325

Committed: https://crrev.com/ffd75360db3fc16ec6b4cb28c5b68b534f8ac55d
Cr-Commit-Position: refs/heads/master@{#426607}
==========

[commit-bot: I haz the power, 2016-10-21 13:22:57]

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