ppapi: PPB_VpnProvider: Implement Resource Stub

ppapi: PPB_VpnProvider: Implement Resource Stub

BUG=506490
Committed: https://crrev.com/0873bcb27ece58d1585a730dc02beb58ddb120e2
Cr-Commit-Position: refs/heads/master@{#398513}

[adrian.belgun, 2016-04-27 14:58:53]

adrian.belgun@intel.com changed reviewers:
+ bbudge@chromium.org

[adrian.belgun, 2016-04-27 14:58:53]

bbudge: Please review the Resource for VpnProvider

[robert.bradford, 2016-05-13 11:04:43]

The CQ bit was checked by robert.bradford@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-13 11:04:56]

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

[commit-bot: I haz the power, 2016-05-13 11:19:50]

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

[commit-bot: I haz the power, 2016-05-13 11:19:51]

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

[robert.bradford, 2016-05-13 13:36:50]

The CQ bit was checked by robert.bradford@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-13 13:37:01]

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

[commit-bot: I haz the power, 2016-05-13 14:13:14]

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

[commit-bot: I haz the power, 2016-05-13 14:13:16]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[robert.bradford, 2016-05-13 14:58:05]

The CQ bit was checked by robert.bradford@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-05-13 14:58:26]

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

[commit-bot: I haz the power, 2016-05-13 16:12:59]

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

[commit-bot: I haz the power, 2016-05-13 16:13:00]

Dry run: This issue passed the CQ dry run.

[bbudge, 2016-05-17 09:50:12]

Sorry for delay. Upside, I am now in your time zone :)

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
File ppapi/proxy/vpn_provider_resource.cc (right):

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:14: #include
"ppapi/shared_impl/tracked_callback.h"
included in header, so redundant

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:23: bind_callback_(NULL),
nullptr

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
File ppapi/proxy/vpn_provider_resource.h (right):

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.h:65: // Send a packet to the browser. This
method is used in both SendPacket() for
nit s/Send/Sends

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.h:70: // Holds user callbacks to invoke later.
nit: Comments don't add much and could be removed.
Here and twice below.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.h:79: // Shared memory buffers used in plugin
<---> browser IPC
same

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.h:83: // Packet buffers
same

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
File ppapi/shared_impl/vpn_provider_util.h (right):

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.h:25: base::SharedMemoryHandle GetHandle();
This doesn't seem to be used yet. It seems undesirable to expose the underlying
shm buffer. Is it necessary?

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.h:31: std::unique_ptr<bool[]> available_;
std::vector<bool> is a little tricky to use but should work here.

[adrian.belgun, 2016-05-17 11:05:29]

> Sorry for delay. Upside, I am now in your time zone :)
No problem. This is excellent news!

Uploaded new patch set with the requested changes.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
File ppapi/proxy/vpn_provider_resource.cc (right):

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:14: #include
"ppapi/shared_impl/tracked_callback.h"
On 2016/05/17 09:50:12, bbudge wrote:
> included in header, so redundant

Done.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:23: bind_callback_(NULL),
On 2016/05/17 09:50:12, bbudge wrote:
> nullptr

Done.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
File ppapi/proxy/vpn_provider_resource.h (right):

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.h:65: // Send a packet to the browser. This
method is used in both SendPacket() for
On 2016/05/17 09:50:12, bbudge wrote:
> nit s/Send/Sends

Done.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.h:70: // Holds user callbacks to invoke later.
On 2016/05/17 09:50:12, bbudge wrote:
> nit: Comments don't add much and could be removed.
> Here and twice below.

Done.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.h:79: // Shared memory buffers used in plugin
<---> browser IPC
On 2016/05/17 09:50:12, bbudge wrote:
> same

Done.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.h:83: // Packet buffers
On 2016/05/17 09:50:12, bbudge wrote:
> same

Done.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
File ppapi/shared_impl/vpn_provider_util.h (right):

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.h:25: base::SharedMemoryHandle GetHandle();
On 2016/05/17 09:50:12, bbudge wrote:
> This doesn't seem to be used yet. It seems undesirable to expose the
underlying
> shm buffer. Is it necessary?

This class is used by both the Resource and the Resource Host. 
This method is used in the Resource Host's Bind() flow to pass the handle to the
Resource.

context.params.AppendHandle(ppapi::proxy::SerializedHandle(
    send_packet_buffer_->GetHandle(),
    kMaxQueuedPackets * kMaxPacketSizeInBytes));

https://codereview.chromium.org/1735473002/diff/240001/content/browser/render...

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.h:31: std::unique_ptr<bool[]> available_;
On 2016/05/17 09:50:12, bbudge wrote:
> std::vector<bool> is a little tricky to use but should work here.

Done.

[bbudge, 2016-05-17 13:34:39]

Since you're adding IPC messages, you'll need a security review. I think they'll
need to see how these are handled in the browser for their review. So you should
either add the host to this CL, or add as much of the resource as you can
without the messages, and include them in the final host CL.

It's fine with me if you land the host and resource in a single CL.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
File ppapi/shared_impl/vpn_provider_util.h (right):

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.h:25: base::SharedMemoryHandle GetHandle();
On 2016/05/17 11:05:29, adrian.belgun wrote:
> On 2016/05/17 09:50:12, bbudge wrote:
> > This doesn't seem to be used yet. It seems undesirable to expose the
> underlying
> > shm buffer. Is it necessary?
> 
> This class is used by both the Resource and the Resource Host. 
> This method is used in the Resource Host's Bind() flow to pass the handle to
the
> Resource.
> 
> context.params.AppendHandle(ppapi::proxy::SerializedHandle(
>     send_packet_buffer_->GetHandle(),
>     kMaxQueuedPackets * kMaxPacketSizeInBytes));
> 
>
https://codereview.chromium.org/1735473002/diff/240001/content/browser/render...

But the host has access to that handle already (to construct the shared buffer).

https://codereview.chromium.org/1931513002/diff/180001/ppapi/proxy/ppapi_mess...
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/1931513002/diff/180001/ppapi/proxy/ppapi_mess...
ppapi/proxy/ppapi_messages.h:2007:
IPC_MESSAGE_CONTROL3(PpapiPluginMsg_VpnProvider_BindReply,
It looks like you're cribbing from WebSocket. The more common pattern is to
group messages with their reply message. I think it's clearer that way because
it reduces the number of commented groups here. Then you would just have
plugin->browser and browser->plugin groups.

https://codereview.chromium.org/1931513002/diff/180001/ppapi/proxy/ppapi_mess...
ppapi/proxy/ppapi_messages.h:2008: uint32_t /* queue_depth */,
queue_size

https://codereview.chromium.org/1931513002/diff/180001/ppapi/shared_impl/vpn_...
File ppapi/shared_impl/vpn_provider_util.cc (right):

https://codereview.chromium.org/1931513002/diff/180001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.cc:23: for (uint32_t it = 0; it < capacity_;
it++) {
nit: s/it/i
since you're indexing, it seems clearer to me at least.

https://codereview.chromium.org/1931513002/diff/180001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.cc:47: return
(void*)((char*)(shm_->memory()) + packet_size_ * id);
c++-style casts for chromium code.

[adrian.belgun, 2016-05-17 15:25:29]

> Since you're adding IPC messages, you'll need a security review. I think
they'll
> need to see how these are handled in the browser for their review. So you
should
> either add the host to this CL, or add as much of the resource as you can
> without the messages, and include them in the final host CL.

Will soon further split a patch.

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
File ppapi/shared_impl/vpn_provider_util.h (right):

https://codereview.chromium.org/1931513002/diff/160001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.h:25: base::SharedMemoryHandle GetHandle();
On 2016/05/17 13:34:39, bbudge wrote:
> On 2016/05/17 11:05:29, adrian.belgun wrote:
> > On 2016/05/17 09:50:12, bbudge wrote:
> > > This doesn't seem to be used yet. It seems undesirable to expose the
> > underlying
> > > shm buffer. Is it necessary?
> > 
> > This class is used by both the Resource and the Resource Host. 
> > This method is used in the Resource Host's Bind() flow to pass the handle to
> the
> > Resource.
> > 
> > context.params.AppendHandle(ppapi::proxy::SerializedHandle(
> >     send_packet_buffer_->GetHandle(),
> >     kMaxQueuedPackets * kMaxPacketSizeInBytes));
> > 
> >
>
https://codereview.chromium.org/1735473002/diff/240001/content/browser/render...
> 
> But the host has access to that handle already (to construct the shared
buffer).

The issue is that this API is tied to the chrome.vpnProvider JS API. 
A connection is bound after it has connected. If the connection gets
disconnected it will also get unbound. Thus, in the app's lifetime, Bind() can
be called multiple times - on every reconnect.

The proposed implementation keeps the shm handle inside the shared buffer class.
Thus, on ulterior Bind() calls, the shm will be retrieved from cache.

The shm will be cleared when the resource gets destroyed.

https://codereview.chromium.org/1931513002/diff/180001/ppapi/proxy/ppapi_mess...
File ppapi/proxy/ppapi_messages.h (right):

https://codereview.chromium.org/1931513002/diff/180001/ppapi/proxy/ppapi_mess...
ppapi/proxy/ppapi_messages.h:2007:
IPC_MESSAGE_CONTROL3(PpapiPluginMsg_VpnProvider_BindReply,
On 2016/05/17 13:34:39, bbudge wrote:
> It looks like you're cribbing from WebSocket. The more common pattern is to
> group messages with their reply message. I think it's clearer that way because
> it reduces the number of commented groups here. Then you would just have
> plugin->browser and browser->plugin groups.

Done.

https://codereview.chromium.org/1931513002/diff/180001/ppapi/proxy/ppapi_mess...
ppapi/proxy/ppapi_messages.h:2008: uint32_t /* queue_depth */,
On 2016/05/17 13:34:39, bbudge wrote:
> queue_size

Done.

https://codereview.chromium.org/1931513002/diff/180001/ppapi/shared_impl/vpn_...
File ppapi/shared_impl/vpn_provider_util.cc (right):

https://codereview.chromium.org/1931513002/diff/180001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.cc:23: for (uint32_t it = 0; it < capacity_;
it++) {
On 2016/05/17 13:34:39, bbudge wrote:
> nit: s/it/i
> since you're indexing, it seems clearer to me at least.

Done.

https://codereview.chromium.org/1931513002/diff/180001/ppapi/shared_impl/vpn_...
ppapi/shared_impl/vpn_provider_util.cc:47: return
(void*)((char*)(shm_->memory()) + packet_size_ * id);
On 2016/05/17 13:34:39, bbudge wrote:
> c++-style casts for chromium code.

Done.

[adrian.belgun, 2016-05-17 16:46:04]

Description was changed from

==========
ppapi: PPB_VpnProvider: Implement Resource

BUG=506490
==========

to

==========
ppapi: PPB_VpnProvider: Implement Resource

BUG=506490
==========

[adrian.belgun, 2016-05-17 16:46:15]

Description was changed from

==========
ppapi: PPB_VpnProvider: Implement Resource

BUG=506490
==========

to

==========
ppapi: PPB_VpnProvider: Implement Resource Stub

BUG=506490
==========

[adrian.belgun, 2016-05-17 17:11:26]

On 2016/05/17 15:25:29, adrian.belgun wrote:
> > Since you're adding IPC messages, you'll need a security review. I think
> they'll
> > need to see how these are handled in the browser for their review. So you
> should
> > either add the host to this CL, or add as much of the resource as you can
> > without the messages, and include them in the final host CL.
> 
> Will soon further split a patch.
> 

Further split patches for easier review. Assigned reviewers to them.

Permission: https://codereview.chromium.org/1985093002/
Service Helper: https://codereview.chromium.org/1988613005/
Resource Stub: https://codereview.chromium.org/1931513002/
Resource Host: https://codereview.chromium.org/1735473002/
Vpn Service: https://codereview.chromium.org/1922873005/

Dependency graph:
Permission, Service Helper, Resource Stub: No dependencies. 
Resource Host: Depends on Permission, Service Helper, Resource Stub.
Vpn Service:   Depends on Service Helper.

[bbudge, 2016-06-06 17:53:03]

Chrome coding standards don't allow commented out code so just remove it for
this CL.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
File ppapi/proxy/vpn_provider_resource.cc (right):

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:27: // SendCreate(BROWSER,
PpapiHostMsg_VpnProvider_Create());
Remove commented out code.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:44: //     OnPluginMsgOnPacketReceived)
Remove

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:64: bind_callback_ = callback;
nit: You should probably also remove this since the call doesn't succeed right
now.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:72: // return PP_OK_COMPLETIONPENDING;
Remove

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:141: // Responds to
PpapiPluginMsg_VpnProvider_OnUnbind
nit: Eliminate comments on implementations of methods. Here and below.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:163: // Post(BROWSER,
PpapiHostMsg_VpnProvider_OnPacketReceivedReply(id));
remove commented out code and comment
Here and below.

[adrian.belgun, 2016-06-07 08:14:10]

>> Chrome coding standards don't allow commented out code
>> so just remove it for this CL.

Uploaded new patch set with clean-up.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
File ppapi/proxy/vpn_provider_resource.cc (right):

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:27: // SendCreate(BROWSER,
PpapiHostMsg_VpnProvider_Create());
On 2016/06/06 17:53:03, bbudge wrote:
> Remove commented out code.

Done.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:44: //     OnPluginMsgOnPacketReceived)
On 2016/06/06 17:53:03, bbudge wrote:
> Remove

Done.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:64: bind_callback_ = callback;
On 2016/06/06 17:53:03, bbudge wrote:
> nit: You should probably also remove this since the call doesn't succeed right
> now.

Done.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:72: // return PP_OK_COMPLETIONPENDING;
On 2016/06/06 17:53:03, bbudge wrote:
> Remove

Done.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:141: // Responds to
PpapiPluginMsg_VpnProvider_OnUnbind
On 2016/06/06 17:53:02, bbudge wrote:
> nit: Eliminate comments on implementations of methods. Here and below.

Done.

https://codereview.chromium.org/1931513002/diff/220001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:163: // Post(BROWSER,
PpapiHostMsg_VpnProvider_OnPacketReceivedReply(id));
On 2016/06/06 17:53:03, bbudge wrote:
> remove commented out code and comment
> Here and below.

Done.

[adrian.belgun, 2016-06-07 08:14:26]

>> Chrome coding standards don't allow commented out code
>> so just remove it for this CL.

Uploaded new patch set with clean-up.

[bbudge, 2016-06-08 01:50:43]

lgtm

[adrian.belgun, 2016-06-08 03:55:42]

The CQ bit was checked by adrian.belgun@intel.com

[commit-bot: I haz the power, 2016-06-08 03:55:57]

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

[commit-bot: I haz the power, 2016-06-08 03:55:58]

This CL has an open dependency (Issue 1988613005 Patch 1). Please resolve the
dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[adrian.belgun, 2016-06-08 04:36:10]

The CQ bit was checked by adrian.belgun@intel.com

[adrian.belgun, 2016-06-08 04:36:11]

The patchset sent to the CQ was uploaded after l-g-t-m from bbudge@chromium.org
Link to the patchset: https://codereview.chromium.org/1931513002/#ps260001
(title: "Remove CL dependency")

[commit-bot: I haz the power, 2016-06-08 04:36:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1931513002/260001

[commit-bot: I haz the power, 2016-06-08 05:22:15]

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

[commit-bot: I haz the power, 2016-06-08 05:22:17]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[adrian.belgun, 2016-06-08 08:55:41]

https://codereview.chromium.org/1931513002/diff/260001/ppapi/proxy/vpn_provid...
File ppapi/proxy/vpn_provider_resource.cc (right):

https://codereview.chromium.org/1931513002/diff/260001/ppapi/proxy/vpn_provid...
ppapi/proxy/vpn_provider_resource.cc:36: const IPC::Message& msg) {
This patch set fails the Windows try bots due to "switch statement contains
'default' but no 'case' labels".
https://msdn.microsoft.com/en-us/library/te26h1tz(v=vs.90).aspx

The next patch set removes the macros and replaces them with:
PluginResource::OnReplyReceived(params, msg);

[adrian.belgun, 2016-06-08 08:56:06]

The CQ bit was checked by adrian.belgun@intel.com

[adrian.belgun, 2016-06-08 08:56:06]

The patchset sent to the CQ was uploaded after l-g-t-m from bbudge@chromium.org
Link to the patchset: https://codereview.chromium.org/1931513002/#ps280001
(title: "Simplify OnReplyReceived. Fix Windows build.")

[commit-bot: I haz the power, 2016-06-08 08:56:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1931513002/280001

[commit-bot: I haz the power, 2016-06-08 10:01:06]

Description was changed from

==========
ppapi: PPB_VpnProvider: Implement Resource Stub

BUG=506490
==========

to

==========
ppapi: PPB_VpnProvider: Implement Resource Stub

BUG=506490
==========

[commit-bot: I haz the power, 2016-06-08 10:01:08]

Committed patchset #15 (id:280001)

[commit-bot: I haz the power, 2016-06-08 10:02:12]

Description was changed from

==========
ppapi: PPB_VpnProvider: Implement Resource Stub

BUG=506490
==========

to

==========
ppapi: PPB_VpnProvider: Implement Resource Stub

BUG=506490

Committed: https://crrev.com/0873bcb27ece58d1585a730dc02beb58ddb120e2
Cr-Commit-Position: refs/heads/master@{#398513}
==========

[commit-bot: I haz the power, 2016-06-08 10:02:13]

Patchset 15 (id:??) landed as
https://crrev.com/0873bcb27ece58d1585a730dc02beb58ddb120e2
Cr-Commit-Position: refs/heads/master@{#398513}