Properly initialize top controls content offset on LayerTreeHost.

Properly initialize top controls content offset on LayerTreeHost.

https://codereview.chromium.org/511253003 added top control content offset
to LayerTreeHost and LayerTreeImpl but didn't sync the two properly on
startup, causing the top controls to hide on navigations. This patch makes
the initial value of the top controls such that the top controls are hidden
(and rely on the initial UpdateTopControlsState from the browser). At the
same time, it also changes LayerTreeImpl's offset to be the content offset,
rather than top controls offset, to match LayerTreeHost.

BUG=412918
Committed: https://crrev.com/55b2f1575955a6c7a55e09990361fb7bf0404565
Cr-Commit-Position: refs/heads/master@{#294824}

[bokan, 2014-09-11 16:00:20]

bokan@chromium.org changed reviewers:
+ aelias@chromium.org

[bokan, 2014-09-11 16:00:20]

PTAL

[aelias_OOO_until_Jul13, 2014-09-11 16:30:17]

Hmm, but this leaves Blink's value still thinking it's 0, which will cause more
subtle problems later.  How about we instead switch top_controls_manager.cc to
initialize controls_top_offset to -top_controls_height and content_top_offset to
0.  Then the next call to UpdateTopControlsState should propagate the delta down
properly.  Hopefully we already get an UpdateTopControlsState very early in
initialization (and if not we can add an extra one of those on the Android
browser side).

[bokan, 2014-09-11 18:23:34]

On 2014/09/11 16:30:17, aelias wrote:
> Hmm, but this leaves Blink's value still thinking it's 0, which will cause
more
> subtle problems later.  How about we instead switch top_controls_manager.cc to
> initialize controls_top_offset to -top_controls_height and content_top_offset
to
> 0.  Then the next call to UpdateTopControlsState should propagate the delta
down
> properly.  Hopefully we already get an UpdateTopControlsState very early in
> initialization (and if not we can add an extra one of those on the Android
> browser side).

Ok, I changed TopControlsManager to set the top offset to -top_controls_height.
Seems
to work so it looks like we do get an UpdateTopControlsState at the right time
(though,
TBH, I'm ignorant of the Browser side here).

I had to move the top controls creation to happen after LayerTreeImpl creation,
since
that's where the controls_top_offset lives. There's also a duplicate
SetDebugState call
that surrounded TopControlsManager creation. As far as I can tell, that should
have no
effect so I removed it.

[aelias_OOO_until_Jul13, 2014-09-11 23:05:54]

OK, now I'm getting confused.  Your first patch sets the LTH offset to max
top_content_offset (bottom coord of URL bar, zero or positive), and your second
patch sets the LTI offset to min top_controls_offset (top coord of URL bar, zero
or negative).  Those have very different results, I'm not sure how they can both
be correct.  Which is the intended range of the field?  And it looks like prior
to this patch, everything was synced to 0 in a nicely aligned way so I'm not
sure what is the synchronization problem you think might be causing this bug.

[aelias_OOO_until_Jul13, 2014-09-11 23:10:37]

OK, I figured it out.  In LTH you're storing the content offset and in LTI
you're storing the controls offset.  Please just switch the semantics of LTI
field to be the content offset and keep initializing it at 0.  Make
top_controls_manager convert up between content offset and controls offset at
time of use.  This should fix the bug and be clearer (I prefer the values in LTI
to be pure copies of those in LTH).

[bokan, 2014-09-12 18:22:08]

On 2014/09/11 23:10:37, aelias wrote:
> OK, I figured it out.  In LTH you're storing the content offset and in LTI
> you're storing the controls offset.  Please just switch the semantics of LTI
> field to be the content offset and keep initializing it at 0.  Make
> top_controls_manager convert up between content offset and controls offset at
> time of use.  This should fix the bug and be clearer (I prefer the values in
LTI
> to be pure copies of those in LTH).

Yah, that's right. Sorry about the confusion; I changed LTI to use
content_offset,
that does make more sense. LTH and LTI now both initialize to content_offset =
0,
i.e. bar hidden. Then during initialization we get sent down an
UpdateTopControlsState.

[aelias_OOO_until_Jul13, 2014-09-12 18:28:47]

lgtm

Please also delete the unused field TopControlsManager::controls_top_offset_
while you're at it, and update the patch description before landing.

[bokan, 2014-09-12 21:25:30]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2014-09-12 21:31:33]

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

[commit-bot: I haz the power, 2014-09-12 21:53:18]

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

[commit-bot: I haz the power, 2014-09-12 21:53:19]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[bokan, 2014-09-15 13:44:01]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2014-09-15 13:45:06]

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

[commit-bot: I haz the power, 2014-09-15 14:49:45]

Committed patchset #5 (id:80001) as f8313d69aea8cabc1eaf1ddef6f524fee6d8c6db

[commit-bot: I haz the power, 2014-09-15 15:08:40]

Patchset 5 (id:??) landed as
https://crrev.com/55b2f1575955a6c7a55e09990361fb7bf0404565
Cr-Commit-Position: refs/heads/master@{#294824}