Show a Google "G" in the omnibox on the New Tab Page.

Show a Google "G" in the omnibox on the New Tab Page.

The "G" shrinks down and fades when focusing the omnibox or scrolling up.

Screenshots of the base and half-expanded state: https://goo.gl/photos/EkCPYSVuMjgHmQrv8


BUG=690981
Review-Url: https://codereview.chromium.org/2683653007
Cr-Commit-Position: refs/heads/master@{#450056}
Committed: https://chromium.googlesource.com/chromium/src/+/b7402e0e149bc2e59d44f5378b86b26d9d35565e

[Bernhard Bauer, 2017-02-10 16:02:50]

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

[commit-bot: I haz the power, 2017-02-10 16:03:19]

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, 2017-02-10 17:05:50]

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

[commit-bot: I haz the power, 2017-02-10 17:05:51]

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

[Bernhard Bauer, 2017-02-10 17:42:52]

Description was changed from

==========
Show a Google "G" in the omnibox on the New Tab Page.

BUG=TBD
==========

to

==========
Show a Google "G" in the omnibox on the New Tab Page.

BUG=690981
==========

[Bernhard Bauer, 2017-02-10 18:03:49]

findbugs

[Bernhard Bauer, 2017-02-10 18:04:27]

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

[commit-bot: I haz the power, 2017-02-10 18:05:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bernhard Bauer, 2017-02-10 18:06:47]

bauerb@chromium.org changed reviewers:
+ mdjones@chromium.org

[Bernhard Bauer, 2017-02-10 18:06:48]

Please review. I still need to add a feature to control this, but I'd be
thankful if you could take an initial look at the location bar / toolbar
changes. Thanks!

[Bernhard Bauer, 2017-02-10 18:38:39]

Description was changed from

==========
Show a Google "G" in the omnibox on the New Tab Page.

BUG=690981
==========

to

==========
Show a Google "G" in the omnibox on the New Tab Page.

The "G" shrinks down and fades when focusing the omnibox or scrolling up.

BUG=690981
==========

[Bernhard Bauer, 2017-02-10 18:44:33]

Description was changed from

==========
Show a Google "G" in the omnibox on the New Tab Page.

The "G" shrinks down and fades when focusing the omnibox or scrolling up.

BUG=690981
==========

to

==========
Show a Google "G" in the omnibox on the New Tab Page.

The "G" shrinks down and fades when focusing the omnibox or scrolling up.

Screenshots of the base and half-expanded state:
https://goo.gl/photos/EkCPYSVuMjgHmQrv8


BUG=690981
==========

[commit-bot: I haz the power, 2017-02-10 18:44:40]

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

[commit-bot: I haz the power, 2017-02-10 18:44:41]

Dry run: This issue passed the CQ dry run.

[mdjones, 2017-02-11 00:32:58]

Looks good, just some nits so far.

https://codereview.chromium.org/2683653007/diff/20001/chrome/android/java/res...
File chrome/android/java/res/layout/location_bar.xml (right):

https://codereview.chromium.org/2683653007/diff/20001/chrome/android/java/res...
chrome/android/java/res/layout/location_bar.xml:16: <ImageView
android:id="@+id/google_g"
This block needs further indentation.

https://codereview.chromium.org/2683653007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarPhone.java
(right):

https://codereview.chromium.org/2683653007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarPhone.java:279:
NewTabPage ntp = getToolbarDataProvider().getNewTabPageForCurrentTab();
Can we isolate all this new logic into a function like the two above? Something
like updateGoogleG()?

[Bernhard Bauer, 2017-02-13 13:58:56]

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

[commit-bot: I haz the power, 2017-02-13 13:59:15]

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, 2017-02-13 14:32:14]

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

[commit-bot: I haz the power, 2017-02-13 14:32:15]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2017-02-13 15:04:05]

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

[commit-bot: I haz the power, 2017-02-13 15:04:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Justin Donnelly, 2017-02-13 15:22:33]

jdonnelly@chromium.org changed reviewers:
+ jdonnelly@chromium.org

[Justin Donnelly, 2017-02-13 15:22:34]

What about the case where the user has chosen another search engine?

[commit-bot: I haz the power, 2017-02-13 16:31:26]

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

[commit-bot: I haz the power, 2017-02-13 16:31:27]

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

[Bernhard Bauer, 2017-02-13 17:17:12]

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

[commit-bot: I haz the power, 2017-02-13 17:17:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bernhard Bauer, 2017-02-13 17:18:01]

On 2017/02/13 15:22:34, Justin Donnelly wrote:
> What about the case where the user has chosen another search engine?

In that case we don't show a search box on the NTP, and the toolbar omnibox
stays at the top. The isLocationBarShownInNTP() check in LocationBarPhone takes
care of that.

https://codereview.chromium.org/2683653007/diff/20001/chrome/android/java/res...
File chrome/android/java/res/layout/location_bar.xml (right):

https://codereview.chromium.org/2683653007/diff/20001/chrome/android/java/res...
chrome/android/java/res/layout/location_bar.xml:16: <ImageView
android:id="@+id/google_g"
On 2017/02/11 00:32:57, mdjones wrote:
> This block needs further indentation.

Done.

https://codereview.chromium.org/2683653007/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarPhone.java
(right):

https://codereview.chromium.org/2683653007/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarPhone.java:279:
NewTabPage ntp = getToolbarDataProvider().getNewTabPageForCurrentTab();
On 2017/02/11 00:32:57, mdjones wrote:
> Can we isolate all this new logic into a function like the two above?
Something
> like updateGoogleG()?

Done.

[Justin Donnelly, 2017-02-13 17:43:28]

On 2017/02/13 17:18:01, Bernhard Bauer wrote:
> On 2017/02/13 15:22:34, Justin Donnelly wrote:
> > What about the case where the user has chosen another search engine?
> 
> In that case we don't show a search box on the NTP, and the toolbar omnibox
> stays at the top. The isLocationBarShownInNTP() check in LocationBarPhone
takes
> care of that.

I see, thanks for the explanation. Can you add a comment to that effect for
others wondering the same thing? The definition of isLocationBarShownInNTP
doesn't (unless I'm missing it somewhere) assert this condition either (that the
NTP only shows a search box if the user's search engine is Google).

[Bernhard Bauer, 2017-02-13 18:24:07]

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

[commit-bot: I haz the power, 2017-02-13 18:24:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bernhard Bauer, 2017-02-13 18:25:27]

bauerb@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Bernhard Bauer, 2017-02-13 18:25:28]

Alexei, could I get an OWNERS review for histograms? Thanks!

On 2017/02/13 17:43:28, Justin Donnelly wrote:
> On 2017/02/13 17:18:01, Bernhard Bauer wrote:
> > On 2017/02/13 15:22:34, Justin Donnelly wrote:
> > > What about the case where the user has chosen another search engine?
> > 
> > In that case we don't show a search box on the NTP, and the toolbar omnibox
> > stays at the top. The isLocationBarShownInNTP() check in LocationBarPhone
> takes
> > care of that.
> 
> I see, thanks for the explanation. Can you add a comment to that effect for
> others wondering the same thing? The definition of isLocationBarShownInNTP
> doesn't (unless I'm missing it somewhere) assert this condition either (that
the
> NTP only shows a search box if the user's search engine is Google).

Done.

[Alexei Svitkine (slow), 2017-02-13 18:27:46]

lgtm

[mdjones, 2017-02-13 18:50:15]

lgtm

[Bernhard Bauer, 2017-02-13 18:54:47]

The CQ bit was checked by bauerb@chromium.org

[commit-bot: I haz the power, 2017-02-13 18:55:04]

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, 2017-02-13 20:04:43]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487012087160410,
"parent_rev": "a182fc4e6a01755fc9b53947f3064e59a39a6fdd", "commit_rev":
"b7402e0e149bc2e59d44f5378b86b26d9d35565e"}

[commit-bot: I haz the power, 2017-02-13 20:05:15]

Description was changed from

==========
Show a Google "G" in the omnibox on the New Tab Page.

The "G" shrinks down and fades when focusing the omnibox or scrolling up.

Screenshots of the base and half-expanded state:
https://goo.gl/photos/EkCPYSVuMjgHmQrv8


BUG=690981
==========

to

==========
Show a Google "G" in the omnibox on the New Tab Page.

The "G" shrinks down and fades when focusing the omnibox or scrolling up.

Screenshots of the base and half-expanded state:
https://goo.gl/photos/EkCPYSVuMjgHmQrv8


BUG=690981

Review-Url: https://codereview.chromium.org/2683653007
Cr-Commit-Position: refs/heads/master@{#450056}
Committed:
https://chromium.googlesource.com/chromium/src/+/b7402e0e149bc2e59d44f5378b86...
==========

[commit-bot: I haz the power, 2017-02-13 20:05:17]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/b7402e0e149bc2e59d44f5378b86...