Might be something a Noogler could help with.

Note that fixing  bug 841271  will help address item (4) above.

(1), (2), (3) are addressed: https://chromium-review.googlesource.com/c/chromium/src/+/1100196

(4) seems to have been addressed by another recent CL.

After enabling the hover ink drop highlight, some clipping/masking logic is apparently cutting off the ink drop highlight but not the ink drop ripple.  This seems wrong, but is probably a separate issue.  I will look into it, but advice is welcome - and let me know if I should file a separate bug for this ink drop rendering inconsistency.

Deleted: InkDropMaskHighlightClipped.png 12.5 KB

	InkDropMaskHighlightClipped.png 12.5 KB View Download

Deleted: InkDropRippleShows.png 13.6 KB

	InkDropRippleShows.png 13.6 KB View Download

1, 2, 3 LGTM. 
The clipping does seem to be another issue. I defer to kylixrd@ on how to proceed. 

Question for pkasting@ : On (1), should the 12 DIP vs 14 DIP be coming from a central source of information that chooses icon sizes, etc. -- or is this from-the-spec info fair game for coding directly to source?  I don't like hard-coding numbers, but it seems common in new_tab_button.cc and I'm unaware of a styles structure or some such source of truth.

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

commit 775707e91ade177c36bbe952abe75c0090420ae8
Author: Orin Jaworski <orinj@chromium.org>
Date: Fri Jun 15 19:36:56 2018

Tweak new tab button icon/ink drop per spec

Use 14x14 DIP icon in touchable mode, change icon color
from COLOR_TAB_TEXT (GG800) to COLOR_BACKGROUND_TAB_TEXT
(GG700), and turn on the ink drop hover highlight.

Bug:  851041 
Change-Id: I6b415d03a39914f7f35003be511b45332d1543f7
Reviewed-on: https://chromium-review.googlesource.com/1100196
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567765}
[modify] https://crrev.com/775707e91ade177c36bbe952abe75c0090420ae8/chrome/browser/ui/views/tabs/new_tab_button.cc

orinj: Is this fixed?

I addressed items 1-3 and 4 was apparently already addressed, so I think this issue is resolved, but it may very well have revealed a new issue as shown in the screenshot.  Should we open a new bug for that and mark this one fixed?  As a matter of principle, I don't like the idea of calling my own bugs 'fixed', I think that should always come from a separate, ideally independent entity; but I am not familiar with the process here yet.

I think the clipping issue in the screenshots of c#4 is reported in  issue 853700 .

Comment 0 item (4) does not appear to be addressed, by code inspection.  For example, https://cs.chromium.org/chromium/src/chrome/browser/ui/views/tabs/new_tab_button.cc?rcl=4429dfc0c28654be26b2e2f3190b482bfae5c58e&l=103 still says 0.14.

Okay, I had seen some changes affecting its color elsewhere in the code recently, but the link is helpful and I will look more into (4) soon. I am working in this code right now on another open issue.

Here are screens of touch and non-touch ink effects after opacity tweaks of Patch Set 2 on https://chromium-review.googlesource.com/c/chromium/src/+/1109459

Deleted: NonTouchWithoutInk.png 11.2 KB

	NonTouchWithoutInk.png 11.2 KB View Download

Deleted: NonTouchHoverHighlightOnly.png 11.6 KB

	NonTouchHoverHighlightOnly.png 11.6 KB View Download

Deleted: NonTouchRippleInProgress.png 11.1 KB

	NonTouchRippleInProgress.png 11.1 KB View Download

Deleted: TouchableRefreshWithoutInk.png 13.5 KB

	TouchableRefreshWithoutInk.png 13.5 KB View Download

Deleted: TouchableRefreshHoverHighlightOnly.png 13.4 KB

	TouchableRefreshHoverHighlightOnly.png 13.4 KB View Download

Deleted: TouchableRefreshRippleInProgress.png 12.7 KB

	TouchableRefreshRippleInProgress.png 12.7 KB View Download

OK, looking at those screenshots and the spec, I think:

* The value passed to set_ink_drop_visible_opacity() is an opacity atop the hovered state, as opposed to atop the background.  So for the non-touch case for example, the CL paints the hover as GG900 10% atop the frame, and then the ripple as GG900 16% atop the hover.

* The spec for non-touch gives a "pressed" state in terms of the opacity against the background, but for touch gives a pressed ripple in terms of the opacity against the hover.  As I asked bettes@ via email (no reply yet), it's not clear if the spec is trying to disable ripples for non-touch, and if so, whether that would be a change from other buttons (which currently have ripples on non-touch).

Here's my suggestion: change the ripple opacity to 8% all the time.  This makes both touch and non-touch exactly match the touch spec.  The net effect on touch is very similar to the non-touch spec anyway (GG900 @16% is close to GG900 @8% atop GG900 @10%), so until we get further clarity from Alan on what the intent of the non-touch design is, this is as close as I think we should try to get.

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

commit 3ee6269d98e83b59bb90c0499eea51abdd0972f1
Author: Orin Jaworski <orinj@chromium.org>
Date: Fri Jun 22 17:28:08 2018

Change new tab button ink drop opacity

New tab button ink drop effect is made less opaque for touch
and more opaque for non-touch interfaces.

Bug:  851041 
Change-Id: I2243a44f3d98899eace803a0681e1b060014e08e
Reviewed-on: https://chromium-review.googlesource.com/1109459
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569680}
[modify] https://crrev.com/3ee6269d98e83b59bb90c0499eea51abdd0972f1/chrome/browser/ui/views/tabs/new_tab_button.cc

bettes: Is there additional work required here?

Leaving open for now since we don't actually match the spec, because I want to understand the intent of the spec (see private email/comment 14).

Word from bettes@ is "looks good"