These are probably cases requiring the theme authors to update the theme itself. +pkasting@ for Win and +shrike@ for Mac to comment.

Is it possible to get (or figure out) the name of the theme?

You can see in screenshot 3, for example, that the buttons are not missing, but the theme author has intentionally set them to a similar color as the toolbar.

This achieves an "etched" sort of look in pre-MD since we draw shadows, but results in unusable buttons in MD.  There's no real way to fix this (detecting and working around this kind of case has its own problems), so theme authors need to update their themes.

ConOps, please tell affected users that they'll need to use a different theme or get the theme author to update their theme to use a different button color.

Re-opening the bug since it's re-appearing for M58. 

Can we get more details on
* Sample themes users are seeing this with
* Confirmation that the problems were not occurring with M57
* Any flags/switches these users have set

melodychu@ - can you please provide more details and possible crash ids or feedback reports? This was listed as an item that has re-appeared last week during gCon weekly Issues report. 

There's discussion in these threads: https://productforums.google.com/forum/#!topic/chrome/03uJl2rrrWU
https://www.reddit.com/r/chrome/comments/67lhh5/was_there_a_chrome_update_recently/

One user says this theme does not work:
https://chrome.google.com/webstore/detail/black-theme-aero/epjeeohjfjgmnfpemiceoglbnnieffoh?hl=en

This is another theme mentioned in a feedback report:
https://chrome.google.com/webstore/detail/late-night/pgbdhkpacgdhfabeceekiafonfkipohm?hl=en

The first link you post says people see better results after resetting to the default theme then readding their old theme.  That would suggest we screwed something up in our color caching maybe.

The second link just says something about colors getting slightly darker, which I'm not sure how much to worry about.

I confirm that "Black Theme - Aero" does not have readable text in background tabs.  "Late Night" is hard to read (but not completely illegible).  I didn't test these pre-M58 to see if something got worse.

I suspect the issues here may be different than the original on this bug.

+CC estade for comments and assigned to bsep who's done work on themes recently.  Bret, maybe you can test M57 vs. M58 here as well as whether there's some kind of color caching issue that would explain the first paragraph above, and file new bugs as needed?

Sure, I'll investigate if anything changed 57->58, or if our color caching has a bug.

jainabhishek@ - please provide a link to a Chrome theme that isn't working for you.

Hello Jason,

I am not impacted by this bug. I am using default theme.
I am part of Product operations (gCon) team and raise bug based on user feedback.

Kameron, do you have new reports from forum about this issue ? 

Yes, I've seen a number of issues related to this but users have commented that reinstalling the theme seems to fix the issue. I also ran into the issue myself and that resolved it (https://chrome.google.com/webstore/detail/super-mario-bros-widescre/kmhijkmdacfhmffldohgadlmjjeobmle?hl=en). I don't have names of the exact themes but the issue seems to be wider spread across a number of themes and not specific themes. Let me know if you need us to loop back with these users anyways to get the names of the themes.

Here are some examples:
https://www.reddit.com/r/chrome/comments/68lbmo/address_bar_icons_missing/
https://productforums.google.com/forum/#!topic/chrome/R7xosTq12Eo
https://productforums.google.com/forum/#!topic/chrome/03uJl2rrrWU
https://productforums.google.com/forum/#!topic/chrome/wNJymGjkMOI
https://productforums.google.com/forum/#!topic/chrome/JHqarPDCvcM

Update: The culprit is my patch here: https://codereview.chromium.org/2663363002. Merely bumping the theme pack version to 47 causes... something to happen and the button tint from the theme isn't applied (so it's dark grey even on a black toolbar). Bumping it again to 48 fixes it, but JUST changing the version should be a effective no-op so this is somewhat worrying. I'm investigating further.

Does anyone know where the code is that wipes out the cached theme when the version changes? I'm having trouble finding it.

Looks like ThemeService::LoadThemePrefs() tries to load the theme from the data pack, which will fail on version mismatch.  In that case this function fails to call set_ready(), and I believe this means later ThemeService::OnExtensionServiceReady() will call MigrateTheme(), which calls BuildFromExtension() to rebuild the data pack.

Okay, I figured it out. This is what happened:

1. http://crrev.com/2667753004 (Tom's patch) lands, which modifies ThemeProperties::OverwritableByUserThemeProperty and bumps the theme version to 46.
2. http://crrev.com/2663363002 (my patch) lands, which does some unrelated stuff and also bumps the theme version to 47.
3. My patch is merged back to M57. When people install M57 their theme version goes from 45 to 47, and so their themes are regenerated.
4. M58 comes out, which includes Tom's patch, but the version is still 47 so no one's patches are regenerated. So I essentially "skipped" Tom's theme version bump.

I'll land a patch that just bumps the theme version to 48, and merge it back to 59 so people get fixed-up sooner. But I have no idea how to avoid this kind of problem in the future. At least it's a really rare situation.

Ah.  We've seen this sort of thing before.

Basically, the rule is, if you're going to merge back a change that bumps a versioned resource to version N, you have to ensure all N-1 and prior changes are also merged.

This sort of gets caught during the merge, because the diff from "46" to "47" should fail to auto-merge correctly, and the fixup point is the point that's supposed to clue in the human to do something better.

But I don't know how to add automated enforcement of this to ensure people actually do it.

Your suggested fix looks good.  I would ask if it can be merged to M58 as well in case we do any more respins of that, since this is pretty bad for a lot of theme users.

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

commit 488babf8ae0aad660733d118f28c37169e1ca784
Author: bsep <bsep@chromium.org>
Date: Sat May 13 21:28:01 2017

Increase theme version to fix M57->M58 upgrade not migrating themes.

This is what happened:
1. http://crrev.com/2667753004 (Tom's patch) lands, which modifies
ThemeProperties::OverwritableByUserThemeProperty and bumps the theme
version to 46.
2. http://crrev.com/2663363002 (my patch) lands, which does some
unrelated stuff and also bumps the theme version to 47.
3. My patch is merged back to M57. When people install M57 their theme
version goes from 45 to 47, and so their themes are regenerated.
4. M58 comes out, which includes Tom's patch, but the version is still
47 so no one's patches are regenerated. So I essentially "skipped" Tom's
theme version bump.

This patch bumps the version again, in order to regenerate everyone's
themes and get them in sync with Tom's patch.

BUG= 652948 

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

[modify] https://crrev.com/488babf8ae0aad660733d118f28c37169e1ca784/chrome/browser/themes/browser_theme_pack.cc

Okay, adding a merge request for both versions. 58 seems like it'd be okay to me too, since it's a very low risk but very high visibility change.

Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

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

Please merge your change to M59 branch 3071 by 4:00 PM PT, Monday (05/15) so we can take it in for next week beta release. Thank you.

Sorry, I wasn't in yesterday. I'll merge it right now.

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

commit fb1a94d396a22cfae2bf8876f7639ac57f31e886
Author: Bret Sepulveda <bsep@chromium.org>
Date: Tue May 16 19:58:50 2017

Increase theme version to fix M57->M58 upgrade not migrating themes.

This is what happened:
1. http://crrev.com/2667753004 (Tom's patch) lands, which modifies
ThemeProperties::OverwritableByUserThemeProperty and bumps the theme
version to 46.
2. http://crrev.com/2663363002 (my patch) lands, which does some
unrelated stuff and also bumps the theme version to 47.
3. My patch is merged back to M57. When people install M57 their theme
version goes from 45 to 47, and so their themes are regenerated.
4. M58 comes out, which includes Tom's patch, but the version is still
47 so no one's patches are regenerated. So I essentially "skipped" Tom's
theme version bump.

This patch bumps the version again, in order to regenerate everyone's
themes and get them in sync with Tom's patch.

BUG= 652948 

Review-Url: https://codereview.chromium.org/2880003003
Cr-Original-Commit-Position: refs/heads/master@{#471606}
Review-Url: https://codereview.chromium.org/2884423002 .
Cr-Commit-Position: refs/branch-heads/3071@{#588}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/fb1a94d396a22cfae2bf8876f7639ac57f31e886/chrome/browser/themes/browser_theme_pack.cc

Tested this issue on Windows-10 and Mac OS 10.12 using chrome latest beta M59-59.0.3071.61 by installing few themes from webstore and including themes provided in the comment #9. Observed no issues in displaying the navigation button when the chrome theme is dark or white. 	

bsep@ Attaching screen-shot for reference, could you please confirm is this the expected behavior of the issue? Able to see navigation buttons but not the tittle of the tabs.

Thanks!

Deleted: Screen Shot 2017-05-17 at 2.17.18 PM.png 222 KB

	Screen Shot 2017-05-17 at 2.17.18 PM.png 222 KB View Download

 bsep@ if possible can we get a theme which we can try I tried almost 10 different themes with older build where the CL wasn't present and still can't reproduce the issue. 

The problem would never occur when newly installing a theme -- only when upgrading Chrome through the two successive versions in question while having the theme already installed.  I wouldn't worry about trying to reproduce this from scratch.

Issue 718282 has been merged into this issue.

We're are not planning any further M58 stable releases. Rejecting merge to M58.

@30: We're not requesting a respin of 58, but I know a few other things have landed in case we end up having to do another respin.  The merge request here was along similar lines: it's not worth shipping a new M58 for this, but if we unexpectedly shipped one for other reasons I'd really like this before M59.

Not resetting merge request so as not to be a jerk; maybe you already considered this line of argument.

Thank you pkasting@. 
Changing back to "Merge-Request-58" just in case if we do respin M58 (at the moment there is no plan to respin)

Rejecting merge to M58 as we're are not planning any further M58 stable releases. Thank you.

Issue 726163 has been merged into this issue.