Move legacyCreateTypeface to SkFontStyle.

Move legacyCreateTypeface to SkFontStyle.

This allows Chromium and Blink to ask for arbitrary font
weights now, making the transition to SkFontMgr easier.

Committed: https://crrev.com/079490eadcb27d78f888124a105b5576e33fc69a
Cr-Commit-Position: refs/heads/master@{#386914}

[bungeman-chromium, 2016-04-11 14:10:58]

The CQ bit was checked by bungeman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-11 14:11:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1877673002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1877673002/20001

[commit-bot: I haz the power, 2016-04-11 14:20:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-11 14:20:05]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[bungeman-chromium, 2016-04-11 15:07:07]

Description was changed from

==========
Move legacyCreateTypeface to SkFontStyle.

COMMIT=false
This currently contains a Skia patch.
==========

to

==========
Move legacyCreateTypeface to SkFontStyle.

COMMIT=false
Currently applies Skia patch from https://crrev.com/1873923002 .
==========

[bungeman-chromium, 2016-04-11 17:38:25]

Description was changed from

==========
Move legacyCreateTypeface to SkFontStyle.

COMMIT=false
Currently applies Skia patch from https://crrev.com/1873923002 .
==========

to

==========
Move legacyCreateTypeface to SkFontStyle.

COMMIT=false
Currently applies Skia patch from https://crrev.com/1873923002 .
After the Skia change lands, the DEPS change will be removed before landing.
==========

[bungeman-chromium, 2016-04-11 17:39:39]

Description was changed from

==========
Move legacyCreateTypeface to SkFontStyle.

COMMIT=false
Currently applies Skia patch from https://crrev.com/1873923002 .
After the Skia change lands, the DEPS change will be removed before landing.
==========

to

==========
Move legacyCreateTypeface to SkFontStyle.

This allows Chromium and Blink to ask for arbitrary font
weights now, making the transition to SkFontMgr easier.

COMMIT=false
Currently applies Skia patch from https://crrev.com/1873923002 .
After the Skia change lands, the DEPS change will be removed before landing.
==========

[bungeman-chromium, 2016-04-11 17:59:11]

bungeman@chromium.org changed reviewers:
+ eae@chromium.org, erg@chromium.org, jln@chromium.org

[Elliot Glaysher, 2016-04-11 18:04:42]

components/font_service lgtm

https://codereview.chromium.org/1877673002/diff/60001/components/font_service...
File components/font_service/public/interfaces/font_service.mojom (right):

https://codereview.chromium.org/1877673002/diff/60001/components/font_service...
components/font_service/public/interfaces/font_service.mojom:11: struct
TypefaceStyle {
one line of whitespace between the enum and the struct.

[eae, 2016-04-11 18:21:55]

LGTM for Blink

[bungeman-chromium, 2016-04-11 18:29:21]

bungeman@chromium.org changed reviewers:
+ dcheng@chromium.org

[bungeman-chromium, 2016-04-11 18:29:22]

https://codereview.chromium.org/1877673002/diff/60001/components/font_service...
File components/font_service/public/interfaces/font_service.mojom (right):

https://codereview.chromium.org/1877673002/diff/60001/components/font_service...
components/font_service/public/interfaces/font_service.mojom:11: struct
TypefaceStyle {
On 2016/04/11 18:04:41, Elliot Glaysher wrote:
> one line of whitespace between the enum and the struct.

Done.

[bungeman-chromium, 2016-04-11 18:41:46]

The CQ bit was checked by bungeman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-11 18:42:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1877673002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1877673002/80001

[dcheng, 2016-04-11 19:51:07]

https://codereview.chromium.org/1877673002/diff/80001/content/common/font_con...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1877673002/diff/80001/content/common/font_con...
content/common/font_config_ipc_linux.cc:79:
request.WriteUInt32(static_cast<uint32_t>(requestedStyle));
How does this work? Isn't SkFontStyle a class?
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/skia/i...

[bungeman-chromium, 2016-04-11 20:32:17]

https://codereview.chromium.org/1877673002/diff/80001/content/common/font_con...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1877673002/diff/80001/content/common/font_con...
content/common/font_config_ipc_linux.cc:79:
request.WriteUInt32(static_cast<uint32_t>(requestedStyle));
On 2016/04/11 19:51:06, dcheng wrote:
> How does this work? Isn't SkFontStyle a class?
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/skia/i...

It is a class, but is explicitly convertible to a uint32_t (since that's
essentially what it is). If it's preferable, we could write
skia::WriteSkFontStyle and skia::ReadSkFontStyle in skia_utils_base, but it
seemed like a lot of work for what was essentially just an uint32_t. However, it
wouldn't be too much work to do if you think it would be better to write it out
per-component and then read it back in per-component.

[dcheng, 2016-04-11 21:06:50]

https://codereview.chromium.org/1877673002/diff/80001/content/common/font_con...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1877673002/diff/80001/content/common/font_con...
content/common/font_config_ipc_linux.cc:79:
request.WriteUInt32(static_cast<uint32_t>(requestedStyle));
On 2016/04/11 20:32:17, bungeman2 wrote:
> On 2016/04/11 19:51:06, dcheng wrote:
> > How does this work? Isn't SkFontStyle a class?
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/skia/i...
> 
> It is a class, but is explicitly convertible to a uint32_t (since that's
> essentially what it is). If it's preferable, we could write
> skia::WriteSkFontStyle and skia::ReadSkFontStyle in skia_utils_base, but it
> seemed like a lot of work for what was essentially just an uint32_t. However,
it
> wouldn't be too much work to do if you think it would be better to write it
out
> per-component and then read it back in per-component.

Right, I see that it just wraps a union that's uint32_t sized. But I don't see
operator uint32_t() defined on SkFontStyle, so I'm wondering how the compiler
ends up performing this conversion.

In any case, I'd prefer to see this written with something more explicit like
base::TimeTicks::ToInternalValue() and base::TimeTicks::FromInternalValue(). We
could just call it ToUint32() and FromUint32() for SkFontStyle.

[bungeman-chromium, 2016-04-11 22:31:11]

https://codereview.chromium.org/1877673002/diff/80001/content/common/font_con...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1877673002/diff/80001/content/common/font_con...
content/common/font_config_ipc_linux.cc:79:
request.WriteUInt32(static_cast<uint32_t>(requestedStyle));
On 2016/04/11 21:06:50, dcheng wrote:
> On 2016/04/11 20:32:17, bungeman2 wrote:
> > On 2016/04/11 19:51:06, dcheng wrote:
> > > How does this work? Isn't SkFontStyle a class?
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/skia/i...
> > 
> > It is a class, but is explicitly convertible to a uint32_t (since that's
> > essentially what it is). If it's preferable, we could write
> > skia::WriteSkFontStyle and skia::ReadSkFontStyle in skia_utils_base, but it
> > seemed like a lot of work for what was essentially just an uint32_t.
However,
> it
> > wouldn't be too much work to do if you think it would be better to write it
> out
> > per-component and then read it back in per-component.
> 
> Right, I see that it just wraps a union that's uint32_t sized. But I don't see
> operator uint32_t() defined on SkFontStyle, so I'm wondering how the compiler
> ends up performing this conversion.

The Skia change linked in the description is being pulled in via DEPS.

> 
> In any case, I'd prefer to see this written with something more explicit like
> base::TimeTicks::ToInternalValue() and base::TimeTicks::FromInternalValue().
We
> could just call it ToUint32() and FromUint32() for SkFontStyle.

Done.

[bungeman-chromium, 2016-04-11 22:31:51]

The CQ bit was checked by bungeman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-11 22:33:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1877673002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1877673002/100001

[bungeman-chromium, 2016-04-11 22:35:10]

bungeman@chromium.org changed reviewers:
+ jam@chromium.org

[bungeman-chromium, 2016-04-11 22:35:11]

jam: hate to bug you for this, but I apparently need a content/OWNER for the
content/child/font_warmup_win_unittest.cc change. Of course, feel free to take a
look at the rest of this as well.

[commit-bot: I haz the power, 2016-04-11 22:42:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-11 22:42:27]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)

[dcheng, 2016-04-12 00:01:24]

https://codereview.chromium.org/1877673002/diff/100001/content/common/font_co...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1877673002/diff/100001/content/common/font_co...
content/common/font_config_ipc_linux.cc:109: *outStyle = reply_style;
I don't think I conveyed my intent clearly: if there are explicit FromUint32()
and ToUint32() methods SkFontStyle (which replace the uint32_t conversion
constructor and operator), then we can just write this as:
  uint32_t reply_style_value;
  if (... || !iter.ReadUInt32(&reply_style_value)) return false;
  if (outStyle)
    *outStyle = SkFontStyle::FromUint32(reply_style_value);

Similarly, the other serialization bit can just look like this:
  request.WriteUint32(requestedStyle.ToUint32());

[jam, 2016-04-12 16:03:44]

On 2016/04/11 22:35:11, bungeman2 wrote:
> jam: hate to bug you for this, but I apparently need a content/OWNER for the
> content/child/font_warmup_win_unittest.cc change.
lgtm
> Of course, feel free to take a
> look at the rest of this as well.

[bungeman-chromium, 2016-04-12 16:35:36]

https://codereview.chromium.org/1877673002/diff/100001/content/common/font_co...
File content/common/font_config_ipc_linux.cc (right):

https://codereview.chromium.org/1877673002/diff/100001/content/common/font_co...
content/common/font_config_ipc_linux.cc:109: *outStyle = reply_style;
On 2016/04/12 00:01:24, dcheng wrote:
> I don't think I conveyed my intent clearly: if there are explicit FromUint32()
> and ToUint32() methods SkFontStyle (which replace the uint32_t conversion
> constructor and operator), then we can just write this as:
>   uint32_t reply_style_value;
>   if (... || !iter.ReadUInt32(&reply_style_value)) return false;
>   if (outStyle)
>     *outStyle = SkFontStyle::FromUint32(reply_style_value);
> 
> Similarly, the other serialization bit can just look like this:
>   request.WriteUint32(requestedStyle.ToUint32());

This is a hard sell to Skia though. The uint32_t conversion constructor and
operator were added in the Skia CL to make this CL simpler so that how to
properly serialize it could go in another CL. However, as you pointed out, it
isn't ideal to hide what's going on that way, and also these 'invisible'
explicit conversions were actually already causing the introduction of bugs in
Skia (my reviewer really didn't like them too). But at least they were
'invisible' and not likely to be used. Adding named ReadUInt32 and FromUInt32
methods is quite visible and rather limiting to future design. It has been and
will be a lot of work to go from SkTypeface::Style to SkFontStyle mostly because
SkTypeface::Style implementation was overly exposed and couldn't be updated. In
other words, Skia has been bitten by fixed sizes before, and doesn't want to
have public methods to expose those sizes, because those public methods will end
up abused by some user of Skia other than Chromium in some strange way.
base::TimeTicks has the advantage that it's a type in an application (rather
than a library).

Is there any particular reason why the current proposal (PS6,
skia/ext/skia_utils_base) is undesirable? It is currently 48/64bits instead of
32bits, but that could be fixed (essentially be ReadUInt32 and FromUInt32 but on
the Chromium side, not constraining SkFontStyle from changing its layout, and
not going to be used for other purposes). Mostly I did it this way because it
seems like it should make the fewest people uncomfortable, and also follows the
example of SkFontIdentity which seems to be fairly similar. On the other hand,
if you're not comfortable with the current proposal I'll try to see if there is
some other way. If adding ReadUInt32 and FromUInt32 to SkFontStyle is the only
way to get this in, I'll try to fight for that, but I'd prefer not to if it can
be avoided.

[dcheng, 2016-04-12 17:11:23]

On 2016/04/12 16:35:36, bungeman2 wrote:
>
https://codereview.chromium.org/1877673002/diff/100001/content/common/font_co...
> File content/common/font_config_ipc_linux.cc (right):
> 
>
https://codereview.chromium.org/1877673002/diff/100001/content/common/font_co...
> content/common/font_config_ipc_linux.cc:109: *outStyle = reply_style;
> On 2016/04/12 00:01:24, dcheng wrote:
> > I don't think I conveyed my intent clearly: if there are explicit
FromUint32()
> > and ToUint32() methods SkFontStyle (which replace the uint32_t conversion
> > constructor and operator), then we can just write this as:
> >   uint32_t reply_style_value;
> >   if (... || !iter.ReadUInt32(&reply_style_value)) return false;
> >   if (outStyle)
> >     *outStyle = SkFontStyle::FromUint32(reply_style_value);
> > 
> > Similarly, the other serialization bit can just look like this:
> >   request.WriteUint32(requestedStyle.ToUint32());
> 
> This is a hard sell to Skia though. The uint32_t conversion constructor and
> operator were added in the Skia CL to make this CL simpler so that how to
> properly serialize it could go in another CL. However, as you pointed out, it
> isn't ideal to hide what's going on that way, and also these 'invisible'
> explicit conversions were actually already causing the introduction of bugs in
> Skia (my reviewer really didn't like them too). But at least they were
> 'invisible' and not likely to be used. Adding named ReadUInt32 and FromUInt32
> methods is quite visible and rather limiting to future design. It has been and
> will be a lot of work to go from SkTypeface::Style to SkFontStyle mostly
because
> SkTypeface::Style implementation was overly exposed and couldn't be updated.
In
> other words, Skia has been bitten by fixed sizes before, and doesn't want to
> have public methods to expose those sizes, because those public methods will
end
> up abused by some user of Skia other than Chromium in some strange way.
> base::TimeTicks has the advantage that it's a type in an application (rather
> than a library).
> 
> Is there any particular reason why the current proposal (PS6,
> skia/ext/skia_utils_base) is undesirable? It is currently 48/64bits instead of
> 32bits, but that could be fixed (essentially be ReadUInt32 and FromUInt32 but
on
> the Chromium side, not constraining SkFontStyle from changing its layout, and
> not going to be used for other purposes). Mostly I did it this way because it
> seems like it should make the fewest people uncomfortable, and also follows
the
> example of SkFontIdentity which seems to be fairly similar. On the other hand,
> if you're not comfortable with the current proposal I'll try to see if there
is
> some other way. If adding ReadUInt32 and FromUInt32 to SkFontStyle is the only
> way to get this in, I'll try to fight for that, but I'd prefer not to if it
can
> be avoided.

No particular reason. From earlier replies I just thought this approach wasn't
actually preferred. But if this is the preferred option, then it's fine with me.

https://codereview.chromium.org/1877673002/diff/120001/skia/ext/skia_utils_ba...
File skia/ext/skia_utils_base.cc (right):

https://codereview.chromium.org/1877673002/diff/120001/skia/ext/skia_utils_ba...
skia/ext/skia_utils_base.cc:53: if (style) {
Seems odd that this is an optional parameter. It doesn't need to be, does it?

(I see the other helpers allow this, but that also seems odd to me)

[bungeman-chromium, 2016-04-12 18:15:39]

https://codereview.chromium.org/1877673002/diff/120001/skia/ext/skia_utils_ba...
File skia/ext/skia_utils_base.cc (right):

https://codereview.chromium.org/1877673002/diff/120001/skia/ext/skia_utils_ba...
skia/ext/skia_utils_base.cc:53: if (style) {
On 2016/04/12 17:11:23, dcheng wrote:
> Seems odd that this is an optional parameter. It doesn't need to be, does it?
> 
> (I see the other helpers allow this, but that also seems odd to me)

I think the original intent was to avoid the locals and the copy in
font_config_ipc_linux.cc. One could just pass 'outFontIdentity' to
ReadSkFontIdentity, it would read in the data (since that always needs to be
done), and if 'outFontIdentity' were nullptr just skip setting 'identity'. It's
possible to see how this evolved from
https://codereview.chromium.org/12391070/diff/1/content/common/font_config_ip...
.

Of course, this never happens because font_config_ipc_linux.cc passes the
address of a local, ReadSkFontIdentity runs, and then if 'outFontIdentity' is
not nullptr a copy happens. Allowing the parameter to be optional was to avoid
that copy and just write directly into 'outFontIdentity' when possible. Oddly,
when this was introduced this wasn't taken advantage of
https://codereview.chromium.org/12391070/diff2/22005:54001/content/common/fon...
.


The documentation in the header file also supports this.

I can remove these checks and assert that the out parameter is not nullptr, or I
could update the calling code to just pass through its own out parameter
pointers (as appears to have been the original intention of doing things this
way), or I could just leave these as-is. Whichever way seems best, I think these
should all be consistent.

[bungeman-chromium, 2016-04-12 18:47:04]

https://codereview.chromium.org/1877673002/diff/120001/skia/ext/skia_utils_ba...
File skia/ext/skia_utils_base.cc (right):

https://codereview.chromium.org/1877673002/diff/120001/skia/ext/skia_utils_ba...
skia/ext/skia_utils_base.cc:53: if (style) {
On 2016/04/12 18:15:38, bungeman2 wrote:
> On 2016/04/12 17:11:23, dcheng wrote:
> > Seems odd that this is an optional parameter. It doesn't need to be, does
it?
> > 
> > (I see the other helpers allow this, but that also seems odd to me)
> 
> I think the original intent was to avoid the locals and the copy in
> font_config_ipc_linux.cc. One could just pass 'outFontIdentity' to
> ReadSkFontIdentity, it would read in the data (since that always needs to be
> done), and if 'outFontIdentity' were nullptr just skip setting 'identity'.
It's
> possible to see how this evolved from
>
https://codereview.chromium.org/12391070/diff/1/content/common/font_config_ip...
> .
> 
> Of course, this never happens because font_config_ipc_linux.cc passes the
> address of a local, ReadSkFontIdentity runs, and then if 'outFontIdentity' is
> not nullptr a copy happens. Allowing the parameter to be optional was to avoid
> that copy and just write directly into 'outFontIdentity' when possible. Oddly,
> when this was introduced this wasn't taken advantage of
>
https://codereview.chromium.org/12391070/diff2/22005:54001/content/common/fon...
> .
> 
> 
> The documentation in the header file also supports this.
> 
> I can remove these checks and assert that the out parameter is not nullptr, or
I
> could update the calling code to just pass through its own out parameter
> pointers (as appears to have been the original intention of doing things this
> way), or I could just leave these as-is. Whichever way seems best, I think
these
> should all be consistent.

To provide an example, taking advantage of the way this is currently written (at
PS7) looks like

--- a/content/common/font_config_ipc_linux.cc
+++ b/content/common/font_config_ipc_linux.cc
@@ -87,28 +87,12 @@ bool FontConfigIPC::matchFamilyName(const char familyName[],
   base::Pickle reply(reinterpret_cast<char*>(reply_buf), r);
   base::PickleIterator iter(reply);
   bool result;
-  if (!iter.ReadBool(&result))
+  if (!iter.ReadBool(&result) || !result)
     return false;
-  if (!result)
-    return false;
-
-  SkString     reply_family;
-  FontIdentity reply_identity;
-  SkFontStyle  reply_style;
-  if (!skia::ReadSkString(&iter, &reply_family) ||
-      !skia::ReadSkFontIdentity(&iter, &reply_identity) ||
-      !skia::ReadSkFontStyle(&iter, &reply_style)) {
-    return false;
-  }
-
-  if (outFontIdentity)
-    *outFontIdentity = reply_identity;
-  if (outFamilyName)
-    *outFamilyName = reply_family;
-  if (outStyle)
-    *outStyle = reply_style;
 
-  return true;
+  return skia::ReadSkString(&iter, outFamilyName) &&
+         skia::ReadSkFontIdentity(&iter, outFontIdentity) &&
+         skia::ReadSkFontStyle(&iter, outStyle);
 }

which is quite a bit of code removed and copies which no longer need to be made
in the common case. I'll have to ask reed why he didn't do this the first time
around (though that was years ago, so probably doesn't remember).

[dcheng, 2016-04-12 18:57:08]

lgtm

I'm OK with PS7. It would be nice to simplify this, but that can happen in a
followup.

[bungeman-chromium, 2016-04-13 02:35:28]

The CQ bit was checked by bungeman@chromium.org

[bungeman-chromium, 2016-04-13 02:35:29]

The patchset sent to the CQ was uploaded after l-g-t-m from erg@chromium.org,
eae@chromium.org, jam@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/1877673002/#ps140001
(title: "Remove DEPS change now that Skia change has landed.")

[commit-bot: I haz the power, 2016-04-13 02:35:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-13 02:35:44]

COMMIT=false detected. CQ is abandoning the patch.

[bungeman-chromium, 2016-04-13 02:36:22]

Description was changed from

==========
Move legacyCreateTypeface to SkFontStyle.

This allows Chromium and Blink to ask for arbitrary font
weights now, making the transition to SkFontMgr easier.

COMMIT=false
Currently applies Skia patch from https://crrev.com/1873923002 .
After the Skia change lands, the DEPS change will be removed before landing.
==========

to

==========
Move legacyCreateTypeface to SkFontStyle.

This allows Chromium and Blink to ask for arbitrary font
weights now, making the transition to SkFontMgr easier.
==========

[bungeman-chromium, 2016-04-13 02:36:33]

The CQ bit was checked by bungeman@chromium.org

[commit-bot: I haz the power, 2016-04-13 02:36:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1877673002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1877673002/140001

[commit-bot: I haz the power, 2016-04-13 04:42:44]

Description was changed from

==========
Move legacyCreateTypeface to SkFontStyle.

This allows Chromium and Blink to ask for arbitrary font
weights now, making the transition to SkFontMgr easier.
==========

to

==========
Move legacyCreateTypeface to SkFontStyle.

This allows Chromium and Blink to ask for arbitrary font
weights now, making the transition to SkFontMgr easier.
==========

[commit-bot: I haz the power, 2016-04-13 04:42:45]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-04-13 04:43:44]

Description was changed from

==========
Move legacyCreateTypeface to SkFontStyle.

This allows Chromium and Blink to ask for arbitrary font
weights now, making the transition to SkFontMgr easier.
==========

to

==========
Move legacyCreateTypeface to SkFontStyle.

This allows Chromium and Blink to ask for arbitrary font
weights now, making the transition to SkFontMgr easier.

Committed: https://crrev.com/079490eadcb27d78f888124a105b5576e33fc69a
Cr-Commit-Position: refs/heads/master@{#386914}
==========

[commit-bot: I haz the power, 2016-04-13 04:43:46]

Patchset 8 (id:??) landed as
https://crrev.com/079490eadcb27d78f888124a105b5576e33fc69a
Cr-Commit-Position: refs/heads/master@{#386914}