CustomElements: adopt node

CustomElements: adopt node

https://dom.spec.whatwg.org/#concept-node-adopt

3.2 For each inclusiveDescendant in node’s shadow-including inclusive
descendants that is a custom element, enqueue a custom element callback
reaction with inclusiveDescendant, callback name "adoptedCallback",
and an empty argument list.

BUG=594918
Committed: https://crrev.com/6b25a0aa041c31e2ebd85bb308672809fc7d940a
Cr-Commit-Position: refs/heads/master@{#409759}

[davaajav, 2016-07-22 09:01:37]

davaajav@google.com changed reviewers:
+ dominicc@chromium.org, kojii@chromium.org

[davaajav, 2016-07-22 09:01:37]

PTL 

JS test for adopt node

[dominicc (has gone to gerrit), 2016-07-22 09:07:58]

Feedback inline.

https://codereview.chromium.org/2170383002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node-expected.txt
(right):

https://codereview.chromium.org/2170383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node-expected.txt:1:
CONSOLE ERROR: line 20: Uncaught ReferenceError: this is not defined
Interesting error; what's going on here?

https://codereview.chromium.org/2170383002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html
(right):

https://codereview.chromium.org/2170383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:25:
assert_equals(invocations.length, 1);
We're writing a lot of things which push records into arrays. Could we write
something that lets us write one readable matcher? like assert_array_equals but
it knows how to inspect its arguments? And then let's use it in all of these
tests. The goal would be to maximize readability.

It would be good to develop it in this test, and in a follow up change move it
to custom-elements-helpers and make all of our tests use it.

https://codereview.chromium.org/2170383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:26:
assert_array_equals(invocations[0].slice(0,2), ['adopted', a], 'the
adoptedCallback should be invoked');
space after ,

https://codereview.chromium.org/2170383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:27:
assert_array_equals(invocations[0][0], [], 'the adoptedCallback should be
invoked with empty argumnets');
spelling: arguments

[davaajav, 2016-07-25 02:58:58]

PTL

[davaajav, 2016-07-25 06:14:02]

PTL

I was making a mistake in the test. adoptedCallback should be enqueued when the
adopting document is NOT the same as the oldDocument.

[dominicc (has gone to gerrit), 2016-07-25 07:56:14]

Comments inline.

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html
(right):

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:9: 
Delete this blank line. You could separate use strict from the comment with one
blank line if you like.

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:11: // 3. If
document is not the same as the oldDocument:
Could we add tests for what happens to exceptions originating from
adoptedCallback?

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:18: let
frame = doc.createElement('iframe');
Look at how test_with_window is implemented and do something similar with two
windows, something like:

promise_test((t) => {
  Promise.all(
    create_window_in_test(t),
    create_window_in_test(t))
  .then(([w1, w2]) => {
    // test code
  });
}, '...');

Promise.all takes a bunch of promises and resolves when they all resolve, with
the results in an array; that thing, [w1, w2] is a destructuring bind that takes
the array and pulls it apart into two elements.

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:24: class X
extends iframeWindow.HTMLElement {
Be consistent with naming; w is an iframe window too, so w/iframeWindow is a bit
jarring. Maybe v,w or windowA, windowB or something.

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:25:
constructor() { super(); }
Again--the implicit one is all you need; you can omit this line.

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:32: },
'adopting a custom element to the same document should enqueue an
adoptedCallback reaction');
It might be worth adding a test that adopting or moving an element within the
same document does not schedule callbacks. I know, it is odd to test a negative,
but we could imagine the predictable bug where we cue that callback on every
tree change and not just adoption.

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js
(right):

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js:37:
function assert_array_equals_callback_invocations(actual, expected, description)
{
Maybe don't put 'array_equals' in the name of this... WDYT about making a class
for recording callback invocations, so the use of the array becomes an internal
implementation detail to it?

[dominicc (has gone to gerrit), 2016-07-28 02:46:00]

Could you change the first line of the description to have a space before
elements, ie "Custom elements", and say a bit more about adopt node? Like, this
adds a test for the adopt node callback.

More comments inline.

https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html
(right):

https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:15:
create_window_in_test(t),
Maybe indent these two lines, two spaces?

https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:26: //
TODO(davaajav):write more specific assertion witj
assert_array_equals_callback_invocations from helpers
spelling: with

https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js
(right):

https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js:37:
function assert_array_equals_callback_invocations(actual, expected, description)
{
I guess if you're not using this in the test here, then you shouldn't add it in
this change.

[dominicc (has gone to gerrit), 2016-08-01 11:09:52]

On 2016/07/28 at 02:46:00, dominicc wrote:
> Could you change the first line of the description to have a space before
elements, ie "Custom elements", and say a bit more about adopt node? Like, this
adds a test for the adopt node callback.
> 
> More comments inline.
> 
>
https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
> File third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html
(right):
> 
>
https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
> third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:15:
create_window_in_test(t),
> Maybe indent these two lines, two spaces?
> 
>
https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
> third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:26: //
TODO(davaajav):write more specific assertion witj
assert_array_equals_callback_invocations from helpers
> spelling: with
> 
>
https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
> File
third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js
(right):
> 
>
https://codereview.chromium.org/2170383002/diff/140001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/custom-elements/spec/resources/custom-elements-helpers.js:37:
function assert_array_equals_callback_invocations(actual, expected, description)
{
> I guess if you're not using this in the test here, then you shouldn't add it
in this change.

Let me know when you're ready for me to take another look at this.

[davaajav, 2016-08-02 07:21:20]

PTL

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html
(right):

https://codereview.chromium.org/2170383002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/custom-elements/spec/adopt-node.html:25:
constructor() { super(); }
On 2016/07/25 at 07:56:13, dominicc wrote:
> Again--the implicit one is all you need; you can omit this line.

Omitting super(); was resulting in "this" being undefined. I guess we should
explicitly call super().

[dominicc (has gone to gerrit), 2016-08-02 07:48:57]

Could you dig into the default constructor more? If I write:

http://jsbin.com/zesejahape/1/edit?html,output (press the "run with JS" button)

it looks like new Y invokes X's constructor.

https://codereview.chromium.org/2170383002/diff/200001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
(right):

https://codereview.chromium.org/2170383002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp:238:
// To report InvalidStateError Exception, when the constructor returns some
differnt object
Spelling: different

https://codereview.chromium.org/2170383002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp:242:

Maybe delete this blank line; since the message and location are
related--they're all for configuring the error.

https://codereview.chromium.org/2170383002/diff/200001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/ElementRareData.cpp (right):

https://codereview.chromium.org/2170383002/diff/200001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/ElementRareData.cpp:89:
visitor->trace(m_V0customElementDefinition);
This field, and the methods above, should be v0Custom... etc.

[davaajav, 2016-08-03 07:42:04]

PTL

[dominicc (has gone to gerrit), 2016-08-03 07:56:23]

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

[commit-bot: I haz the power, 2016-08-03 07:56:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominicc (has gone to gerrit), 2016-08-03 07:56:57]

Great work; this is very close. Some comments inline.

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
(right):

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp:141:
keepAlive(array, 4, adoptedCallback, definition->m_adoptedCallback,
scriptState);
Not important but might be nice to put these in a consistent order to above,
which is consistent with the spec, etc. Needless variation makes people wonder
if there's a reason behind it.

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp
(right):

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLElementCustom.cpp:68:
element->setCustomElementDefinition(definition);
What do you think about setting the state to custom in
setCustomElementDefinition? Because we should never be "custom" without a
definition.

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Element.cpp:1959: void
Element::setCustomElementDefinition(CustomElementDefinition* definition)
I don't think we should allow setting the definition to nullptr.

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Element.h (right):

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Element.h:52: class
V0CustomElementDefinition;
Whoever changed this one forgot to keep them in alphabetical order; could you do
it?

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/ElementRareData.h (right):

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/ElementRareData.h:180:
Member<V0CustomElementDefinition> m_v0customElementDefinition;
As above, treat v0 like a word, so this should be m_v0CustomElementDefinition;
note the big "C".

Koji mentioned storing these pointers in one field, because you can never have
both at once. But I think it is safer to have them separate this way, because
mixing up the type of a pointer is a classic cause of security bugs.

Could you put a TODO on m_v0CustomElementDefinition to remove this field when v0
custom elements are deprecated? If someone sees the "duplicate" fields they
should be placated knowing one of them is going away soon.

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/TreeScopeAdopter.cpp (right):

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/TreeScopeAdopter.cpp:133: if
(node.isElementNode()) {
I believe the custom element state bits actually live in NodeFlags? See:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Node....

I think you can actually call getCustomElementState on Node, and only *then* do
toElement. This may be a tiiiiiiiiny bit faster.

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/custom/CustomElement.cpp (right):

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/custom/CustomElement.cpp:8: #include
"core/dom/ElementRareData.h"
Why is this necessary here?

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/custom/V0CustomElement.cpp (right):

https://codereview.chromium.org/2170383002/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/custom/V0CustomElement.cpp:114:
element->v0setCustomElementDefinition(definition);
Call this v0SetCustomElementDefinition; ie treat v0 like a word.

[haraken, 2016-08-03 08:03:09]

bindings/ LGTM

[commit-bot: I haz the power, 2016-08-03 08:16:38]

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

[commit-bot: I haz the power, 2016-08-03 08:16:39]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[davaajav, 2016-08-03 11:31:12]

davaajav@google.com changed reviewers:
+ haraken@chromium.org

[davaajav, 2016-08-03 11:31:14]

PTL

https://codereview.chromium.org/2170383002/diff/260001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/2170383002/diff/260001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Element.cpp:1959: void
Element::setCustomElementDefinition(CustomElementDefinition* definition)
I am not sure about not allowing setting the definition to nullptr. Is
"!definition" on the line 1961 not sufficient for checking whether definition is
set to nullptr? Please let me know if I am missing something.

[davaajav, 2016-08-04 07:59:30]

PTL

[dominicc (has gone to gerrit), 2016-08-04 08:04:06]

lgtm

LGTM, just one nit inline.

https://codereview.chromium.org/2170383002/diff/280001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/2170383002/diff/280001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Element.cpp:1961: if (!hasRareData() &&
!definition)
I think we should just DCHECK(definition) instead.

[davaajav, 2016-08-04 08:17:50]

PTL

[dominicc (has gone to gerrit), 2016-08-04 08:37:35]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2016-08-04 08:37:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dominicc@chromium.org
Link to the patchset: https://codereview.chromium.org/2170383002/#ps300001
(title: "inline DCHECK(definition) in Element.cpp")

[commit-bot: I haz the power, 2016-08-04 08:37:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-04 08:48:14]

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

[commit-bot: I haz the power, 2016-08-04 08:48:18]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[davaajav, 2016-08-04 09:11:29]

The CQ bit was checked by davaajav@google.com

[davaajav, 2016-08-04 09:11:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dominicc@chromium.org
Link to the patchset: https://codereview.chromium.org/2170383002/#ps320001
(title: "merge conflict resolution")

[commit-bot: I haz the power, 2016-08-04 09:11:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-04 10:41:52]

Committed patchset #17 (id:320001)

[commit-bot: I haz the power, 2016-08-04 10:44:52]

Description was changed from

==========
CustomElements: adopt node

https://dom.spec.whatwg.org/#concept-node-adopt

3.2 For each inclusiveDescendant in node’s shadow-including inclusive
descendants that is a custom element, enqueue a custom element callback
reaction with inclusiveDescendant, callback name "adoptedCallback",
and an empty argument list.

BUG=594918
==========

to

==========
CustomElements: adopt node

https://dom.spec.whatwg.org/#concept-node-adopt

3.2 For each inclusiveDescendant in node’s shadow-including inclusive
descendants that is a custom element, enqueue a custom element callback
reaction with inclusiveDescendant, callback name "adoptedCallback",
and an empty argument list.

BUG=594918

Committed: https://crrev.com/6b25a0aa041c31e2ebd85bb308672809fc7d940a
Cr-Commit-Position: refs/heads/master@{#409759}
==========

[commit-bot: I haz the power, 2016-08-04 10:44:53]

Patchset 17 (id:??) landed as
https://crrev.com/6b25a0aa041c31e2ebd85bb308672809fc7d940a
Cr-Commit-Position: refs/heads/master@{#409759}