[Contextual Search] Adds basic support for narrow Search Panel.

[Contextual Search] Adds basic support for narrow Search Panel.

This CL adds infrastructure support for custom sized
ContenViews, via ContentViewClient.

This CL also adds basically capability for rendering
narrow version of the Search Panel, which will be
used on devices with wide screens.

Moreover, this CL also adjusts our Event handling so
MotionEvents are properly offset to the new ContentView
location, and adjusts the code that checks whether a
particular coordinate is inside the Search Panel or one
of its components.

Finally, this CL also fixes a couple of problems where
the very bottom of the SERP wasn't visible, and fixed
the promotion to a Tab to be seamless even when the
SERP is scrolled.

BUG=510198
BUG=510200
BUG=510205
BUG=510206
Committed: https://crrev.com/4e30e1a1406bede7bfc0197a0fb3b2d056287e5d
Cr-Commit-Position: refs/heads/master@{#338973}

[pedro (no code reviews), 2015-07-14 21:45:00]

pedrosimonetti@chromium.org changed reviewers:
+ dtrainor@chromium.org

[pedro (no code reviews), 2015-07-14 21:45:00]

dtrainor@chromium.org: As we discussed offline, please take a look at this
change.

aurimas@ and donnd@: You don't have to review this change, but feel free to do
it if you want. I'm cc'ing you guys so you know what were the required changes
to support the Narrow Panel.

[donnd, 2015-07-14 22:24:12]

donnd@google.com changed reviewers:
+ donnd@google.com

[donnd, 2015-07-14 22:24:13]

LGTM from my perspective!

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java
(right):

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java:968:
contentViewCore.onPhysicalBackingSizeChanged(width, height);
Should this be called even when the width and height are not changed?

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/eventfilter/ContextualSearchEventFilter.java
(right):

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/eventfilter/ContextualSearchEventFilter.java:361:
final float contentViewOffsetXPx = mSearchPanel.getOffsetX() / mPxToDp;
Should this called method be named getSearchContentViewOffsetX?

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

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:964:
// the Toolbar and Base Page might be offset outside the screen. This means the
rendere
typo (rendere)

[David Trainor- moved to gerrit, 2015-07-14 23:12:01]

lgtm with nits.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java
(right):

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java:968:
contentViewCore.onPhysicalBackingSizeChanged(width, height);
On 2015/07/14 22:24:12, Donn wrote:
> Should this be called even when the width and height are not changed?

I think internally it checks the values and ignores calls with the same width
and height.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
(right):

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java:275:
private boolean isYCoordinateInsideBasePage(float x, float y) {
Should this still have Y in the name?  It seems to check both X and Y.  Same
with the others.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/eventfilter/ContextualSearchEventFilter.java
(right):

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/eventfilter/ContextualSearchEventFilter.java:361:
final float contentViewOffsetXPx = mSearchPanel.getOffsetX() / mPxToDp;
On 2015/07/14 22:24:13, Donn wrote:
> Should this called method be named getSearchContentViewOffsetX?

+1

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

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:866:
mSearchPanelDelegate.getMaximumHeightPx(), MeasureSpec.EXACTLY);
Just to confirm, this only means the search content view core height right?  Not
the panel which includes the top bar?  I don't know if this is a current bug or
something that we should look into that might be masked by the toolbar show/hide
stuff.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:965:
// will consider the ContentView height to be the fullscreen height minus the
Toolbar
This might be fixed if setting the height in getDesiredHeightMeasureSpec()
returned the height - the search box height and this pretended there was no
toolbar instead of pretending there is.

[pedro (no code reviews), 2015-07-15 00:19:17]

Thanks guys, please take another look.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java
(right):

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java:968:
contentViewCore.onPhysicalBackingSizeChanged(width, height);
On 2015/07/14 23:12:00, David Trainor wrote:
> On 2015/07/14 22:24:12, Donn wrote:
> > Should this be called even when the width and height are not changed?
> 
> I think internally it checks the values and ignores calls with the same width
> and height.

I tried to keep the previous behavior, so whenever the
onPhysicalBackingSizeChanged() used to be called, we will call it now with the
new code. Keeping the way it is.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java:968:
contentViewCore.onPhysicalBackingSizeChanged(width, height);
On 2015/07/14 23:12:00, David Trainor wrote:
> On 2015/07/14 22:24:12, Donn wrote:
> > Should this be called even when the width and height are not changed?
> 
> I think internally it checks the values and ignores calls with the same width
> and height.

Acknowledged.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java
(right):

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/contextualsearch/ContextualSearchPanel.java:275:
private boolean isYCoordinateInsideBasePage(float x, float y) {
On 2015/07/14 23:12:00, David Trainor wrote:
> Should this still have Y in the name?  It seems to check both X and Y.  Same
> with the others.

Good catch! Thanks!

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/eventfilter/ContextualSearchEventFilter.java
(right):

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/eventfilter/ContextualSearchEventFilter.java:361:
final float contentViewOffsetXPx = mSearchPanel.getOffsetX() / mPxToDp;
On 2015/07/14 23:12:00, David Trainor wrote:
> On 2015/07/14 22:24:13, Donn wrote:
> > Should this called method be named getSearchContentViewOffsetX?
> 
> +1

Done.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/eventfilter/ContextualSearchEventFilter.java:361:
final float contentViewOffsetXPx = mSearchPanel.getOffsetX() / mPxToDp;
On 2015/07/14 22:24:13, Donn wrote:
> Should this called method be named getSearchContentViewOffsetX?

Good point. Also added separate method for the width and height of the
ContentView.

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

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:866:
mSearchPanelDelegate.getMaximumHeightPx(), MeasureSpec.EXACTLY);
On 2015/07/14 23:12:01, David Trainor wrote:
> Just to confirm, this only means the search content view core height right? 
Not
> the panel which includes the top bar?  I don't know if this is a current bug
or
> something that we should look into that might be masked by the toolbar
show/hide
> stuff.

To be clear, we we're returning the full size, including the toolbar, which is
kinda confusing, I agree. The tricky thing is that we need to tell the system
that the toolbar is visible (or is pretending to be), so that the Tab promotion
happens seamlessly.

Based on our offline chat, I changed the code to only add the client if not in
fullscreen mode. Also, now we send different values to the
updateTopControlsState() depending whether or not we are in fullscreen mode.
This way we keep the complicated part isolated.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:964:
// the Toolbar and Base Page might be offset outside the screen. This means the
rendere
On 2015/07/14 22:24:13, Donn wrote:
> typo (rendere)

Done.

https://codereview.chromium.org/1237913002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java:965:
// will consider the ContentView height to be the fullscreen height minus the
Toolbar
On 2015/07/14 23:12:00, David Trainor wrote:
> This might be fixed if setting the height in getDesiredHeightMeasureSpec()
> returned the height - the search box height and this pretended there was no
> toolbar instead of pretending there is.

We are doing that now for the non-fullscreen case (narrow Panel). We still need
it for the fullscreen case though.

[donnd, 2015-07-15 00:25:39]

I'm not seeing the new patch set, but lgtm based on your replies.

[pedro (no code reviews), 2015-07-15 00:36:14]

Oops... forgot to upload the patch.

[pedro (no code reviews), 2015-07-15 17:56:07]

The CQ bit was checked by pedrosimonetti@chromium.org

[pedro (no code reviews), 2015-07-15 17:56:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, donnd@google.com
Link to the patchset: https://codereview.chromium.org/1237913002/#ps60001
(title: "Sync & rebase")

[commit-bot: I haz the power, 2015-07-15 17:57:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1237913002/60001

[commit-bot: I haz the power, 2015-07-15 22:08:36]

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

[commit-bot: I haz the power, 2015-07-15 22:08:37]

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

[pedro (no code reviews), 2015-07-16 00:21:49]

The CQ bit was checked by pedrosimonetti@chromium.org

[pedro (no code reviews), 2015-07-16 00:21:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, donnd@google.com
Link to the patchset: https://codereview.chromium.org/1237913002/#ps80001
(title: "Adding Finch flag & fixing test")

[commit-bot: I haz the power, 2015-07-16 00:23:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1237913002/80001

[commit-bot: I haz the power, 2015-07-16 00:29:39]

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

[commit-bot: I haz the power, 2015-07-16 00:29:39]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

(exceeded global retry quota)

[pedro (no code reviews), 2015-07-16 00:39:45]

The CQ bit was checked by pedrosimonetti@chromium.org

[pedro (no code reviews), 2015-07-16 00:39:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, donnd@google.com
Link to the patchset: https://codereview.chromium.org/1237913002/#ps100001
(title: "Fixing another bug")

[commit-bot: I haz the power, 2015-07-16 00:40:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1237913002/100001

[commit-bot: I haz the power, 2015-07-16 00:48:15]

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

[commit-bot: I haz the power, 2015-07-16 00:48:16]

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

[pedro (no code reviews), 2015-07-16 01:01:58]

The CQ bit was checked by pedrosimonetti@chromium.org

[commit-bot: I haz the power, 2015-07-16 01:02:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1237913002/100001

[commit-bot: I haz the power, 2015-07-16 01:07:50]

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

[commit-bot: I haz the power, 2015-07-16 01:07:51]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

(exceeded global retry quota)

[pedro (no code reviews), 2015-07-16 01:34:28]

The CQ bit was checked by pedrosimonetti@chromium.org

[pedro (no code reviews), 2015-07-16 01:34:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org, donnd@google.com
Link to the patchset: https://codereview.chromium.org/1237913002/#ps120001
(title: "Sync & rebase")

[commit-bot: I haz the power, 2015-07-16 01:35:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1237913002/120001

[commit-bot: I haz the power, 2015-07-16 02:34:00]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-07-16 02:34:57]

Patchset 7 (id:??) landed as
https://crrev.com/4e30e1a1406bede7bfc0197a0fb3b2d056287e5d
Cr-Commit-Position: refs/heads/master@{#338973}