I think other OS versions are OK, just 10.10 is failing ATM.

Will disable these tests for now per IM conversation with tapted.

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

commit 7e2e998b236a8e3827d4d6290e766a5c0bdd69f3
Author: Peter Kasting <pkasting@chromium.org>
Date: Fri Dec 08 03:42:00 2017

Disable failing tests.

Bug:  793184 
Change-Id: I49068c13d9a28f60ca437b6e17438d9e0fa0ec35
TBR: tapted
Reviewed-on: https://chromium-review.googlesource.com/816069
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522702}
[modify] https://crrev.com/7e2e998b236a8e3827d4d6290e766a5c0bdd69f3/ui/views/controls/styled_label_unittest.cc

I need to dig in more, but this is probably because styled_label.cc uses gfx::ElideRectangleText, so it's possible for the typesetting decisions to mismatch since r522617

I think the smart, long-term thing to do is to stop using ElideRectangleText in StyledLabel. It makes a gfx::RenderText for each *word* in order to calculate the width of that word and whether it will fit on the current line. That's going to be really slow.

Not just that, but StyledLabel calls ElideRectangleText for each line, asking for any remaining lines to be typeset and split into lines - egads. So some pathological case like n words that each occupy an entire line will be O(n*n) calling into some really complex code in gfx::RenderText.

That's some complex code to update though. If there is some severe fallout, I may need to revert that or look for a quicker fix, e.g., by passing a RenderText type to gfx::ElideRectangleText.


Ideal approach.. we could probably do something like render all links using a transparent color, since a views::Link will appear on top of it. And just have a single RenderText instance. no views::Label child views -- just views::Link.


But the original plan was just to migrate the calls using gfx::ElideRectangleText for *Cocoa* code over to a native typesetter, then switch the defaults in gfx's text_elider.h. That may be a more realistic approach.

If there other are serious problems I'll need to revert r522617 and do that first. RenderTextMac and RenderTextHarfbuzz should agree "most of the time" though. If that's not the case, we at least need to get an idea of the scope of the problems.

Thanks for looking into this failure, I'm glad this is the only problem so far.
It'd be nice if StyledLabel functionality was folded into Label/RenderText.

Relatedly, wutao@ may soon extend StyledLabel to support embedding arbitrary Views, so decorated accelerator keys (like the <kbd> HTML tag) can be shown in paragraph form. I really like the idea of simply rendering invisible text, but that wouldn't work for arbitrary views. Maybe RenderText could reserve fixed space amid text for overlay views? Maybe we could learn from Blink.

Anyway, using the correct typesetter in ElideRectangleText definitely seems like a more approachable solution.

msw@, as we discussed offline, while inserting arbitrary Views in StyledLabel is possible, there still some different cases and details need to handle.
1. If the insert view have different height, we want the label on one line is center aligned, so this requires a "dry_run" for each line first, and then assign the correct bounds to each Labels/Views.
2. We can not use gfx::ElideRectangleText to calculate whether the "word" can fit on the current line, because the "word" will be replaced by the "arbitrary View". Therefore we need a different method to calculate it.
3. Reserve a fixed space amid text sounds work, but we also need to reserve the Height as well to center align the text on one line with the overlay views. but the max height only know after one dry run.

Base on the above, I prefer ATM to just fork and modify the StyledLabel::CalculateAndDoLayout() for the use in the View/Label layout of Keyboard Shortcut Viewer, instead to complicate current StyledLabel code and reduce possibly impact if introducing any calculation error.

Forking StyledLabel is a bummer, RenderText reserving fixed space could be nice.
At least the forked StyledLabel will always use Harfbuzz (for ChromeOS).
Sorry for tangential discussion on this bug!

Thanks msw@. I created an issue 793414 for further discussion and implementation.

tapted@ This issue is marked as RB-Beta for M65, could you please let us know is there any latest update available on this issue?

Thanks!

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

commit aa6cafdb584215e0158560cadec23b56a1751925
Author: Trent Apted <tapted@chromium.org>
Date: Wed Jan 10 23:42:01 2018

Make gfx::ElideRectangleText() and url_formatter::ElideHost() typesetter-aware.

gfx::ElideRectangleText() is used on Mac both for native (system-
driven) UI and for Chrome secondary UI. The former is used for
system notifications and will be typeset by CoreText. The latter is
typeset by Harfbuzz.

This CL introduces gfx::ElideRectangleTextForNativeUi(), whose only
consumer at this stage is the Mac message center, which pops native
system notifications.

Nothing else but MacViews was relying on gfx::ElideRectangleText() on
Mac, so switch it over to Harfbuzz.

There *is* one other caller: ExtensionIconPlaceholder::Draw(), which
draws a single letter to a canvas with DrawStringRectWithFlags().
gfx::Canvas::DrawStringRectWithFlags() may call ElideRectangleText() in
some codepaths, so switch DrawStringRectWithFlags() over to Harfbuzz
as well so it's consistent. (In practice, the single letter would
could never elide, so this doesn't change much).

Ensure the rest of cocoa/notification_controller.mm is using the
correct typesetter. This requires url_formatter::ElideHost() to also
be typesetter-aware.

TBR=dtrainor@chromium.org

Bug:  793184 ,  798927 ,  799300 
Change-Id: Ibccb740f97a833c9701b25037d69b0369f732028
Reviewed-on: https://chromium-review.googlesource.com/856016
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528483}
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/chrome/browser/download/download_item_model.cc
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/chrome/browser/ui/cocoa/download/download_item_cell.mm
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/chrome/browser/ui/cocoa/download/md_download_item_view.mm
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/components/url_formatter/elide_url.cc
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/components/url_formatter/elide_url.h
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/ui/gfx/canvas_skia.cc
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/ui/gfx/render_text.cc
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/ui/gfx/render_text.h
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/ui/gfx/text_constants.h
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/ui/gfx/text_elider.cc
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/ui/gfx/text_elider.h
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/ui/message_center/cocoa/notification_controller.mm
[modify] https://crrev.com/aa6cafdb584215e0158560cadec23b56a1751925/ui/views/controls/styled_label_unittest.cc

r528483 renabled the tests, and the waterfall liked them.

But the whackamole with Apple's changing system fonts continues in  Issue 801029 . Now gfx_unittests, failing only on 10.11
  - TextEliderTest.ElideRectangleTextPunctuation
  - TextEliderTest.ElideRectangleTextLongWords

I'm pretty sure this is just a test harness that needs updating though.