+pkasting

I want to avoid painting a completely opaque color because it would break themes like in the attached image.  I've set the stroke color to black with 50% alpha.  So it appears slightly green over the tabstrip and slightly red over inactive tabs.  If we were to use an opaque stroke color, we'd get an odd green color over the inactive tabs.

Attaching horrible paint mock of this with an opaque green stroke.

I dunno, to me this looks reasonable.

Deleted: Untitled.png 6.1 KB

	Untitled.png 6.1 KB View Download

I'm worried about the GTK theme which relies on setting the alpha:
https://cs.chromium.org/chromium/src/chrome/browser/ui/libgtkui/gtk_ui.cc?rcl=6567e305dace61fd4d48fb7f7ad23f81250faa30&l=364

Also I've already created a patch and it wasn't complex to fix:
https://chromium-review.googlesource.com/c/chromium/src/+/1167895

In theory even if we use the GTK theme we're supposed to have background tab colors the same as the frame color.  If we're not doing that, we did something wrong.

I reviewed the patch.  It unfortunately causes other bustage.  It's possible to fix that bustage by instead changing how we draw here, but I still think the opaque separator route is better.

> even if we use the GTK theme we're supposed to have background tab colors the same as the frame color

Ah, ok.  Currently in the GTK theme it's possible for the tab background color to be different to the tabstrip color.

But I'll change the background tab color to always match the tabstrip color.  I actually like it better that way.  And with that changed, I would agree that the opaque separator is a better solution.

You mean, to match the frame color?

Yeah.  Those are always the same color on GTK.

The frame color and tabstrip color, that is.

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

commit 2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Mon Aug 13 23:13:44 2018

[Refresh] Ensure the tab stroke color is opaque

Also change the GTK theme background tab color to match the frame color.

BUG= 872383 
R=pkasting
TBR=sky

Change-Id: I4f1f7e5acf3d989047d565d73964e88306dc97ab
Reviewed-on: https://chromium-review.googlesource.com/1168513
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582737}
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/libgtkui/gtk_ui.cc
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/chrome_browser_main_extra_parts_views.cc
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/frame/browser_non_client_frame_view.h
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.cc
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.h
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/new_tab_button.cc
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/new_tab_button.h
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/tab_controller.h
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/tab_strip.h
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/tab_strip_controller.h
[modify] https://crrev.com/2d4420008d2c3a2cb08ecfb4be6a997fc10df7b7/chrome/browser/ui/views/tabs/tab_unittest.cc

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

commit 7cae60a2e09f4117fa74536618f4e0617b4f23b8
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Tue Aug 14 19:59:46 2018

[Merge to M69] [Refresh] Ensure the tab stroke color is opaque

> Also change the GTK theme background tab color to match the frame color.
>
> BUG= 872383 
> R=pkasting
> TBR=sky

BUG= 872383 
TBR=pkasting,sky
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Change-Id: I4f1f7e5acf3d989047d565d73964e88306dc97ab
Reviewed-on: https://chromium-review.googlesource.com/1168513
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#582737}
Reviewed-on: https://chromium-review.googlesource.com/1175058
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#626}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/libgtkui/gtk_ui.cc
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/chrome_browser_main_extra_parts_views.cc
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/frame/browser_non_client_frame_view.h
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/browser_tab_strip_controller.h
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.cc
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/fake_base_tab_strip_controller.h
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/new_tab_button.cc
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/new_tab_button.h
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/tab.cc
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/tab_controller.h
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/tab_strip.cc
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/tab_strip.h
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/tab_strip_controller.h
[modify] https://crrev.com/7cae60a2e09f4117fa74536618f4e0617b4f23b8/chrome/browser/ui/views/tabs/tab_unittest.cc

The merge CL was approved on  bug 853841 .

Revision 7cae60a2e09f4117fa74536618f4e0617b4f23b8 was merged to refs/branch-heads/3497 branch with no merge approval from a TPM! 

Please explain why this change was merged to the branch!
 

 This merge was approved here - https://bugs.chromium.org/p/chromium/issues/detail?id=853841#c58. Pls see comment #13. Thank you.

Removing "Merge-Without-Approval" label based on comment #15.