"Bootstrap" a toolkit-views Typography spec.

chrome/browser/ui/views/harmony/chrome_typography.h

Index: chrome/browser/ui/views/harmony/chrome_typography.h
diff --git a/chrome/browser/ui/views/harmony/chrome_typography.h b/chrome/browser/ui/views/harmony/chrome_typography.h
new file mode 100644
index 0000000000000000000000000000000000000000..c3eb70b477d7d6299336fc5bfd0128e17b856473
--- /dev/null
+++ b/chrome/browser/ui/views/harmony/chrome_typography.h
@@ -0,0 +1,58 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_HARMONY_CHROME_TYPOGRAPHY_H_
+#define CHROME_BROWSER_UI_VIEWS_HARMONY_CHROME_TYPOGRAPHY_H_
+
+#include "base/macros.h"
+#include "ui/views/style/typography.h"
+#include "ui/views/style/typography_provider.h"
+
+enum ChromeTextContext {
+  // Headline text. Usually 20pt. Never multi-line.
+  CONTEXT_HEADLINE = views::typography::VIEWS_TEXT_CONTEXT_END + 1,

[Peter Kasting, 2017/03/17 02:26:00]

Nit: No need for + 1, and it's confusing (2 places)

[tapted, 2017/03/17 10:33:10]

Done.

+
+  // "Body 1". Usually 13pt.
+  CONTEXT_DIALOG_MESSAGE,

[Peter Kasting, 2017/03/17 02:26:00]

The one discomfort I have with putting "DIALOG" in these names is that it
implies they should not be used outside dialogs, and I don't know whether that's
true.  Something generic like "Body 1" might actually make sense if we think
these values should be applied to all UI in views, dialog or no.  It also makes
it easier to map code directly to mocks, though it makes it harder to read the
code in the absence of those mocks and get as much semantic meaning out of it.

There's no real need to get this perfect now as we can change it later, but I
wanted to raise this so you could consider the tradeoffs.

[tapted, 2017/03/17 10:33:10]

How about BODY_TEXT_LARGE, BODY_TEXT_SMALL? (done)

(I think that gets me over the lack of intrinsic documentation in Body1/Body2,
although LARGE/SMALL feels somewhat reminiscent of the ResourceBundle constants
that get abused..)

[Peter Kasting, 2017/03/17 20:59:39]

Yeah, I hear your concerns.  What if we called the first one just BODY_TEXT or
BODY_TEXT_NORMAL, to suggest that people should default to it if they don't know
what to do?

[tapted, 2017/03/20 07:33:35]

The specs don't really settle on one or the other as what's "normal". The way
I've interpreted it is that 13pt=Body1 tends to get used more often for the
"message" that appears at the top of a dialog. And 12pt=Body2 gets used more for
"instructions" or for text that needs to blend with control labels.

+
+  // "Body 2". Usually 12pt.
+  CONTEXT_DIALOG_TEXT_SMALL,

[Peter Kasting, 2017/03/17 02:25:59]

Nit: "MESSAGE" above and "TEXT" here seem like they should be the same word? 
Maybe change to use TEXT for both?

[tapted, 2017/03/17 10:33:10]

(see previous comment)

+
+  // ResourceBundle::SmallFont (11 pt).

[Peter Kasting, 2017/03/17 02:26:00]

Nit: If this is "DEPRECATED", talk about what that means, why it's here, and
what code that uses this should generally do instead (if there is any guiding
principle).

[tapted, 2017/03/17 10:33:10]

Done.

+  CONTEXT_DEPRECATED_SMALL,
+};
+
+enum ChromeTextStyle {
+  // Secondary text. May be lighter than views::typography::PRIMARY.
+  STYLE_SECONDARY = views::typography::VIEWS_TEXT_STYLE_END + 1,
+
+  // "Hint" text, usually a line that gives context to something more important.
+  STYLE_HINT,
+
+  // A solid shade of red.
+  STYLE_RED,
+
+  // A solid shade of green.
+  STYLE_GREEN,
+
+  // Used to draw attention to a web domain.

[Peter Kasting, 2017/03/17 02:26:00]

Without reading the code, I'd have asked: Does this mean a site/link/URL? 
"Domain" is a very specific part of a URL, so its use here makes me uncertain
about the use of this.  I imagine future readers might also have this question.

Also, "draw attention to" could mean just about anything.  Does that mean people
shouldn't use this if they're not trying to "draw attention"?  Same question on
extensions below.

For both of these reasons, I wonder if we should just say these two are
STYLE_EMPHASIZED or something.  The other values seem very specific and hard to
use or generalize.

Are these expected to be long-lived contexts, or are they effectively
deprecated?

[tapted, 2017/03/17 10:33:10]

I like STYLE_EMPHASIZED for now - it's a little too much like "this is bold",
but I guess that's what it is :/. There isn't a bold font in the spec (except
for button titles on Windows), so this is _probably_ going away. One of the
places where this is used doesn't have a mock - (hostname - first one at
http://crbug.com/700196 ). The other doesn't have a mock either - and I couldn't
figure out how to invoke it... and I just realised why! You can't. It's dead
code.

IDC_CREATE_SHORTCUTS is #ifdefed out on Mac in browser_command_controller.cc --
it's always disabled.

On other platforms, it's only added to app_menu_model.cc when
`extensions::util::IsNewBookmarkAppsEnabled()` is false, but the definition of
that is

bool IsNewBookmarkAppsEnabled() {
#if defined(OS_MACOSX)
  return base::CommandLine::ForCurrentProcess()->HasSwitch(
      switches::kEnableNewBookmarkApps);
#else
  return !base::CommandLine::ForCurrentProcess()->HasSwitch(
      switches::kDisableNewBookmarkApps);
#endif
}

I think we have fully launched this on non-mac, so that flag can be made
mac-only. Filed http://crbug.com/702551



Anyway, I wanted to steer clear of "STYLE_EMPHASIZED" since we might want to
highlight a hostname in some other manner under harmony - i.e. not necessarily
by making it bold, and not necessarily the same way other things are emphasized.
But we can worry about that later.

+  STYLE_WEB_DOMAIN,
+
+  // Used to draw attention to an extension name.
+  STYLE_EXTENSION_NAME,
+};
+
+// TypographyProvider that provides pre-Harmony fonts in Chrome.
+class LegacyTypographyProvider : public views::DefaultTypographyProvider {
+ public:
+  LegacyTypographyProvider() = default;
+
+  // DefaultTypographyProvider:
+  const gfx::FontList& GetFont(int text_context, int text_style) const override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LegacyTypographyProvider);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_HARMONY_CHROME_TYPOGRAPHY_H_