Issue 614673 has been merged into this issue.

Sorry, I missed that this bug is filed to track a separate remaining issue from 612114

Isn't this the general compressed screenshot --> rendered view transition issue?

No, this one is about showing the outline of the top omnibox when switching to the native NTP (which doesn't show the omnibox) from a web page (which does show it).

I can only reproduce this (both Stable and Canary) if I swipe the toolbar to navigate between an NTP and non-NTP tab.

Specifically, I briefly see the toolbar shadow, when transitioning from normal tab to NTP, and no toolbar shadow when transitioning from NTP to normal tab (both is incorrect).

So, as Bernhard mentioned on chat, it seems like the toolbar keeps its previous state during transition.

Ah, you can also get the omnibox shadow to show up if the NTP you switch to is "old enough", i.e. when it takes a bit longer to for the compositing layer to be created.

Again, both on Stable and all other channels.

+mdjones, who might also be interested in this bug :)

Ah yes, I am familiar with this one. I think this is because the tab doesn't have a concept of whether or not a page is native until it is loaded. I can look into a fix for this.

Hi, this is on top of our list of product excellence UI issues. It would be really good to get this fixed in M53.

What are the next steps necessary here?

Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Hi Bernhard and Matthew, who of you is working on this? Comment 23 confused me :)

Hey Matthew, 

are you actively working on this? If so, I'd assign the bug to you. If not, I'd be happy to work on it, but it might be good to sync briefly so I can get some more context about the toolbar and compositor :)

Whoops. Yeah, I started working on it. The approach I started working on was making the toolbar shadow drawn by the compositor in all cases except where an android view is being drawn over it. It looks like I may be trying a slightly different approach since there are a lot of edge cases.

I have two patches that fix this problem:

https://codereview.chromium.org/2161803002/
https://codereview.chromium.org/2158853004/

The general idea is that there is more control over when the compositor toolbar shadow is visible. Unfortunately these changes make the shadow flicker more than it currently does in certain situations (i.e. swiping the toolbar).

Ultimately I'd like the toolbar shadow to be drawn by the compositor 100% of the time, but that is a much larger project.

A somewhat related issue that I've noticed is that the toolbar buttons are the wrong color when switching between a dark themed tab and a light tab (either non-themed or themed in a light color). I think that's the same underlying problem: The toolbar is drawn by the compositor as a dynamic resource backed by the single ToolbarControlContainer, so all tabs get the same bitmap, which has either dark drawables or light drawables. That would need a separate fix though, and at that point I'm not quite sure what would be left of the toolbar to draw :)

Yeah, I've noticed that as well. As the toolbar requirements are becoming more complex, some of the old hacks are not holding up :\. I'm trying to address them as they pop up.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a86eb14b66aec353ca5f12980f53f5c7935371b5

commit a86eb14b66aec353ca5f12980f53f5c7935371b5
Author: mdjones <mdjones@chromium.org>
Date: Wed Jul 27 17:21:42 2016

Add needsToolbarShadow to NativePage interface

This method will allow native pages to notify the compositor whether
or not it needs the toolbar shadow drawn.

BUG=614690

Review-Url: https://codereview.chromium.org/2161803002
Cr-Commit-Position: refs/heads/master@{#408165}

[modify] https://crrev.com/a86eb14b66aec353ca5f12980f53f5c7935371b5/chrome/android/java/src/org/chromium/chrome/browser/FrozenNativePage.java
[modify] https://crrev.com/a86eb14b66aec353ca5f12980f53f5c7935371b5/chrome/android/java/src/org/chromium/chrome/browser/NativePage.java
[modify] https://crrev.com/a86eb14b66aec353ca5f12980f53f5c7935371b5/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkPage.java
[modify] https://crrev.com/a86eb14b66aec353ca5f12980f53f5c7935371b5/chrome/android/java/src/org/chromium/chrome/browser/ntp/IncognitoNewTabPage.java
[modify] https://crrev.com/a86eb14b66aec353ca5f12980f53f5c7935371b5/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/a86eb14b66aec353ca5f12980f53f5c7935371b5/chrome/android/java/src/org/chromium/chrome/browser/ntp/RecentTabsPage.java
[modify] https://crrev.com/a86eb14b66aec353ca5f12980f53f5c7935371b5/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebDiagnosticsPage.java
[modify] https://crrev.com/a86eb14b66aec353ca5f12980f53f5c7935371b5/chrome/android/junit/src/org/chromium/chrome/browser/ntp/NativePageFactoryTest.java

Hi Matt, 

I see the CL on the bug, but I can also still reproduce this in the latest Canary. What's the status of this bug?

Thanks,

Patrick

It's not fully fixed yet -- the CL above is one of two (?) CLs necessary for that.

I've been focusing on the toolbar shadow flickering and haven't looked at the omnibox in the middle of the page. There are a lot of corner-cases I need to make sure work.

Hi Matt,

M54 BP is just around the corner :). Did you get a chance to look into this further?

Thanks

Patrick

Yeah, I have two fixes, each has their own issues:

The first tries to use the compositor shadow whenever possible. This solution resolves almost all of the flickering that the toolbar shadow previously did but draws under the refresh spinner with pull-to-refresh. The find-in-page shadow also does not appear.
https://codereview.chromium.org/2228823003/

The second only solves this problem specifically but increases the amount of flickering done by the shadow.
https://codereview.chromium.org/2158853004/

Hi Matt,

bubbling this up again. Were you able to fix this in M54? If not shall we re-target this to M55?

Thanks

Patrick

Hi Matt,

bubbling this up again. Were you able to fix this in M54? If not shall we re-target this to M55?

Thanks

Patrick

Wasn't able to work out all the bugs for 54; will try for 55.

Hey, time is flying :). M55 BP is tomorrow - how do we look?

Thanks Matt!

Hey Matt,

bubbling this up. Did this get fixed in M55 or M56? :)

Thanks

Patrick

Unfortunately this is not going to get done any time soon. There are other things I am working on that interfere with that patch (which still had bugs).

I guess this is still going to be relevant in a future UX world. So, since we're now getting started on M57 work, I wanted to see where this is in your priorities? (assuming you are the right one to own this)

Unfortunately still no. This is not going to get any love from me until chrome home is done.

Stupid question: Will the issue go away if Chrome Home is enabled?

I think so -- if Chrome Home is not going to modify the omnibox in the same way the current NTP does, this should not happen. (Famous last words...)

Yes, since the NTP will only show inside the bottom sheet for Chrome Home and is the only page that needs to remove the shadow, this will not be a problem.

Any code I had is not obsolete; Chrome Home has priority. Marking as won't fix.

Re-opening per some offline discussions, the shadow overdraw is more noticeable with Chrome Modern so we're going to re-open this (see video).

I'm removing the old labels since we're well past the releases referenced. I think UX would love this for 63, but we've made no eng promises given the complexity.

Deleted: chrome_home_toolbar_flicker.mp4 11.3 MB

	chrome_home_toolbar_flicker.mp4 11.3 MB View Download

Lowering priority following comment #50. Initially targeting M63.