Separate host desktop related functionality into remoting.HostDesktop

Separate host desktop related functionality into remoting.HostDesktop

remoting.ClientPluginImpl has grown organically over the years and become
a dumping ground of all functionality related to the plugin.

This CL moves all  host desktop related implementation from
remoting.ClientPluginImpl into remoting.ClientPlugin.HostDesktopImpl.

BUG=457890
NOTRY=true
TEST=Unit test passed locally and manually verified ShrinkToFit, bumpScrolling and resizeToClient works

Committed: https://crrev.com/87368c7af39e47c717e56e5436b510b1cb3a5eaa
Cr-Commit-Position: refs/heads/master@{#317187}

[kelvinp, 2015-02-18 02:28:39]

kelvinp@chromium.org changed reviewers:
+ garykac@chromium.org, jamiewalch@chromium.org

[kelvinp, 2015-02-18 02:29:47]

Patchset #2 (id:20001) has been deleted

[kelvinp, 2015-02-18 02:31:51]

PTAL

https://codereview.chromium.org/929323003/diff/1/remoting/webapp/browser_test...
File remoting/webapp/browser_test/mock_client_plugin.js (right):

https://codereview.chromium.org/929323003/diff/1/remoting/webapp/browser_test...
remoting/webapp/browser_test/mock_client_plugin.js:153:
remoting.MockClientPlugin.HostDesktop = function() {
Jamie: Although we don't run jscompile over this file, it's still helpful to
annotate
important type information such as @implements. If nothing else, it makes it
clearer that this file is defining two classes.

Done.

https://codereview.chromium.org/929323003/diff/1/remoting/webapp/crd/js/host_...
File remoting/webapp/crd/js/host_desktop.js (right):

https://codereview.chromium.org/929323003/diff/1/remoting/webapp/crd/js/host_...
remoting/webapp/crd/js/host_desktop.js:22: /** @return {boolean} */
When defining an interface, more comments are almost always better than fewer.
Please add comments to the methods and events defined by the interface.

Done.

[Jamie, 2015-02-18 23:06:20]

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/browser_...
File remoting/webapp/browser_test/mock_client_plugin.js (right):

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/browser_...
remoting/webapp/browser_test/mock_client_plugin.js:191: return false;
Nit: Indentation.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/browser_...
remoting/webapp/browser_test/mock_client_plugin.js:201: width, height,
device_scale) {
s/device_scale/dpi/

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/browser_...
remoting/webapp/browser_test/mock_client_plugin.js:205:
this.raiseEvent(remoting.ClientPlugin.HostDesktop.Events.sizeChanged);
How does the event recipient know the new size? It seems like we should raise
the event with |this| as a data member, otherwise the handler needs an
independent reference to the HostDesktop. It can be a follow-up CL though.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_plugin.js (right):

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin.js:87: * @return {boolean} True if the
plugin support the specified feature.
We need to be very careful about the definitions of Feature vs. Capability. I
think this interface is probably the right place to make the distinction,
probably in the comments for this method and the next one.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_plugin_host_desktop_impl.js (right):

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:26:
remoting.ClientPlugin.HostDesktopImpl = function(plugin, postMessageCallback) {
The fact that you need an explicit reference to the plugin and also a function
for posting a message to it is a red flag. If the plugin is intrinsically
needed, then there's no point abstracting its postMessage method. However, I
don't think it is needed if you adopt some of my other suggestions.

Then, if this is no longer tied so closely to the ClientPlugin implementation,
it doesn't need to be a nested class any more.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:42:
this.defineEvents(Object.keys(remoting.HostDesktop.Events));
s/Object.keys/base.values/

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:71: * @param {number}
device_scale
The API declares this parameter as |dpi|, and it refers to the DPI to set on the
host. |device_scale| is used elsewhere to refer to the scale (ie, dpi/96) of the
client device.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:92:
remoting.ClientPlugin.HostDesktopImpl.prototype.onSetCapabilities = function(
I think this method is only used to set hasResizeRateLimit, so if you get rid of
that, this method can go as well.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:104:
remoting.ClientPlugin.HostDesktopImpl.prototype.onSizeUpdated = function(
This method is not declared on the interface and should take width, height and
dpis explicitly. Decoding them here couples this class to ClientPlugin more
tightly than it needs to be.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:130: var rect = {};
Optional: Elsewhere, we have code to force left, right, width and height to be
integers. It might make sense for that to happen here.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_desktop.js (right):

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_desktop.js:23:
remoting.HostDesktop.prototype.hasResizeRateLimit = function() {};
I don't think you need this. It was important to avoid sending too many resize
events to hosts when the user drags the window to resize it, but all hosts have
supported this since M29 (https://chromiumcodereview.appspot.com/15927033), so I
think we can retire it and simplify this interface.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_desktop.js:26:
remoting.HostDesktop.prototype.isResizable = function() {};
I'm not sure you need this either. Some Linux hosts don't support resize, but I
think they just ignore the message. The only reason for having it exposed at the
API level is if we wanted to customize the UI for this case, but I don't think
we do that.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_desktop.js:47: * @param {number} width
I think |width| and |height| are in DIPs, correct? The interface should make
that explicit.

[kelvinp, 2015-02-19 20:58:16]

PTAL

Excuse me for the rebase to merge with Gary's changes.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/browser_...
File remoting/webapp/browser_test/mock_client_plugin.js (right):

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/browser_...
remoting/webapp/browser_test/mock_client_plugin.js:191: return false;
On 2015/02/18 23:06:19, Jamie wrote:
> Nit: Indentation.

Done.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/browser_...
remoting/webapp/browser_test/mock_client_plugin.js:201: width, height,
device_scale) {
On 2015/02/18 23:06:19, Jamie wrote:
> s/device_scale/dpi/

The parameter is about device scale (1.0) instead of DPI (96)

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/browser_...
remoting/webapp/browser_test/mock_client_plugin.js:205:
this.raiseEvent(remoting.ClientPlugin.HostDesktop.Events.sizeChanged);
On 2015/02/18 23:06:19, Jamie wrote:
> How does the event recipient know the new size? It seems like we should raise
> the event with |this| as a data member, otherwise the handler needs an
> independent reference to the HostDesktop. It can be a follow-up CL though.

Done.  In order to hook this event, the client already need a reference to
|this| so I don't think it will be a big problem.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_plugin.js (right):

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin.js:87: * @return {boolean} True if the
plugin support the specified feature.
On 2015/02/18 23:06:19, Jamie wrote:
> We need to be very careful about the definitions of Feature vs. Capability. I
> think this interface is probably the right place to make the distinction,
> probably in the comments for this method and the next one.

Acknowledged.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_plugin_host_desktop_impl.js (right):

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:26:
remoting.ClientPlugin.HostDesktopImpl = function(plugin, postMessageCallback) {
On 2015/02/18 23:06:19, Jamie wrote:
> The fact that you need an explicit reference to the plugin and also a function
> for posting a message to it is a red flag. If the plugin is intrinsically
> needed, then there's no point abstracting its postMessage method. However, I
> don't think it is needed if you adopt some of my other suggestions.
> 
> Then, if this is no longer tied so closely to the ClientPlugin implementation,
> it doesn't need to be a nested class any more.

As discussed, the intent of this class is to separate the functionality of the
plugin into logical groups.  I think it makes sense for this class to live under
the ClientPlugin namespace, so that the reader knows that this it is closely
related to the ClientPlugin.

postMessageCallback is passed in as I don't want the other clients of
clientPlugin to call postMessage() on clientPlugin.
Alternatively, this could be done via a private interface, but would probably be
an overkill for now.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:42:
this.defineEvents(Object.keys(remoting.HostDesktop.Events));
On 2015/02/18 23:06:19, Jamie wrote:
> s/Object.keys/base.values/

Done.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:71: * @param {number}
device_scale
On 2015/02/18 23:06:19, Jamie wrote:
> The API declares this parameter as |dpi|, and it refers to the DPI to set on
the
> host. |device_scale| is used elsewhere to refer to the scale (ie, dpi/96) of
the
> client device.

Done.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:92:
remoting.ClientPlugin.HostDesktopImpl.prototype.onSetCapabilities = function(
On 2015/02/18 23:06:19, Jamie wrote:
> I think this method is only used to set hasResizeRateLimit, so if you get rid
of
> that, this method can go as well.

Done.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:104:
remoting.ClientPlugin.HostDesktopImpl.prototype.onSizeUpdated = function(
On 2015/02/18 23:06:19, Jamie wrote:
> This method is not declared on the interface and should take width, height and
> dpis explicitly. Decoding them here couples this class to ClientPlugin more
> tightly than it needs to be.

This class is under the clientPlugin namespace and is by design coupled.
As long as we have a well defined type for the ClientPluginMessage, I think it
should be fine here.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:130: var rect = {};
On 2015/02/18 23:06:19, Jamie wrote:
> Optional: Elsewhere, we have code to force left, right, width and height to be
> integers. It might make sense for that to happen here.

Done.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_desktop.js (right):

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_desktop.js:23:
remoting.HostDesktop.prototype.hasResizeRateLimit = function() {};
On 2015/02/18 23:06:19, Jamie wrote:
> I don't think you need this. It was important to avoid sending too many resize
> events to hosts when the user drags the window to resize it, but all hosts
have
> supported this since M29 (https://chromiumcodereview.appspot.com/15927033), so
I
> think we can retire it and simplify this interface.

Done.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_desktop.js:26:
remoting.HostDesktop.prototype.isResizable = function() {};
On 2015/02/18 23:06:19, Jamie wrote:
> I'm not sure you need this either. Some Linux hosts don't support resize, but
I
> think they just ignore the message. The only reason for having it exposed at
the
> API level is if we wanted to customize the UI for this case, but I don't think
> we do that.

Yes, I think it would be nice if we can expose that functionality in the UI in
the future.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_desktop.js:47: * @param {number} width
On 2015/02/18 23:06:19, Jamie wrote:
> I think |width| and |height| are in DIPs, correct? The interface should make
> that explicit.

Yes.  Comments updated.

[Jamie, 2015-02-19 22:44:04]

LGTM once my comments are addressed.

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_plugin.js (right):

https://codereview.chromium.org/929323003/diff/40001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin.js:87: * @return {boolean} True if the
plugin support the specified feature.
On 2015/02/19 20:58:15, kelvinp wrote:
> On 2015/02/18 23:06:19, Jamie wrote:
> > We need to be very careful about the definitions of Feature vs. Capability.
I
> > think this interface is probably the right place to make the distinction,
> > probably in the comments for this method and the next one.
> 
> Acknowledged.

Actually, I was suggesting that you document it in this CL :) I'm not sure it's
still necessary if you get rid of hasCapability from this interface, but if you
keep both, then I think you need to make it clear what the difference is.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/base/js/...
File remoting/webapp/base/js/window_shape.js (left):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/base/js/...
remoting/webapp/base/js/window_shape.js:82: }
Apologies for the churn, but now I see the code removed, I think I was wrong
about moving it. It's mostly needed to handle client UI callbacks that return
non-integer values (for example, because they measure DOM elements, which can
have fractional coordinates). Forcing the shape reported by the host desktop to
have integer coordinates is not going to help with that, so I think this part of
the change should be reverted.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/browser_...
File remoting/webapp/browser_test/mock_client_plugin.js (right):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/browser_...
remoting/webapp/browser_test/mock_client_plugin.js:200: deviceScale) {
Nit: When the method name is so long that you can only fit one parameter per
line, it's often more readable to split after "=".

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_plugin_host_desktop_impl.js (right):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:38:
this.hasResizeRateLimit_ = false;
You don't need this (or the accessor) anymore.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_plugin_impl.js (right):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_impl.js:561:
remoting.ClientPluginImpl.prototype.hasCapability = function(capability) {
I don't think you need this any more.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_impl.js:910: * @param {Object}
objectLiteral
|objectLiteral| doesn't describe what this parameter is. I suggest just calling
it |message|.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_desktop.js (right):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_desktop.js:45: * @param {number} width The width of
the desktop in DIP.
s/DIP/DIPs/

[kelvinp, 2015-02-19 23:09:29]

New patchsets have been uploaded after l-g-t-m from jamiewalch@chromium.org

[kelvinp, 2015-02-19 23:35:46]

Patchset #4 (id:80001) has been deleted

[kelvinp, 2015-02-19 23:36:20]

FYI

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/base/js/...
File remoting/webapp/base/js/window_shape.js (left):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/base/js/...
remoting/webapp/base/js/window_shape.js:82: }
On 2015/02/19 22:44:04, Jamie wrote:
> Apologies for the churn, but now I see the code removed, I think I was wrong
> about moving it. It's mostly needed to handle client UI callbacks that return
> non-integer values (for example, because they measure DOM elements, which can
> have fractional coordinates). Forcing the shape reported by the host desktop
to
> have integer coordinates is not going to help with that, so I think this part
of
> the change should be reverted.

Done.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/browser_...
File remoting/webapp/browser_test/mock_client_plugin.js (right):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/browser_...
remoting/webapp/browser_test/mock_client_plugin.js:200: deviceScale) {
On 2015/02/19 22:44:04, Jamie wrote:
> Nit: When the method name is so long that you can only fit one parameter per
> line, it's often more readable to split after "=".

Done.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_plugin_host_desktop_impl.js (right):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_host_desktop_impl.js:38:
this.hasResizeRateLimit_ = false;
On 2015/02/19 22:44:04, Jamie wrote:
> You don't need this (or the accessor) anymore.

Done.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_plugin_impl.js (right):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_impl.js:561:
remoting.ClientPluginImpl.prototype.hasCapability = function(capability) {
On 2015/02/19 22:44:04, Jamie wrote:
> I don't think you need this any more.

I am keeping this function as it still would be useful for other Impl classes to
query for capabilities.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_plugin_impl.js:910: * @param {Object}
objectLiteral
On 2015/02/19 22:44:04, Jamie wrote:
> |objectLiteral| doesn't describe what this parameter is. I suggest just
calling
> it |message|.

Done.

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_desktop.js (right):

https://codereview.chromium.org/929323003/diff/60001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_desktop.js:45: * @param {number} width The width of
the desktop in DIP.
On 2015/02/19 22:44:04, Jamie wrote:
> s/DIP/DIPs/

Done.

[kelvinp, 2015-02-19 23:36:25]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2015-02-19 23:36:36]

The CQ bit was unchecked by kelvinp@chromium.org

[kelvinp, 2015-02-19 23:52:28]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2015-02-19 23:53:41]

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

[commit-bot: I haz the power, 2015-02-19 23:55:11]

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

[commit-bot: I haz the power, 2015-02-19 23:55:12]

Failed to apply patch for remoting/webapp/browser_test/mock_client_plugin.js:
While running git apply --index -3 -p1;
  error: patch failed: remoting/webapp/browser_test/mock_client_plugin.js:151
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: remoting/webapp/browser_test/mock_client_plugin.js: patch does not
apply

Patch:       remoting/webapp/browser_test/mock_client_plugin.js
Index: remoting/webapp/browser_test/mock_client_plugin.js
diff --git a/remoting/webapp/browser_test/mock_client_plugin.js
b/remoting/webapp/browser_test/mock_client_plugin.js
index
f2bb3ceed664b25a6f60b88aabf62a1c791f39f1..09858e0a179fa932916d3dadf39278a710e2c480
100644
--- a/remoting/webapp/browser_test/mock_client_plugin.js
+++ b/remoting/webapp/browser_test/mock_client_plugin.js
@@ -151,6 +151,7 @@ remoting.MockClientPlugin.prototype.setFetchPinHandler =
 /**
  * @constructor
  * @implements {remoting.HostDesktop}
+ * @extends {base.EventSourceImpl}
  */
 remoting.MockClientPlugin.HostDesktop = function() {
   /** @private */
@@ -163,7 +164,7 @@ remoting.MockClientPlugin.HostDesktop = function() {
   this.yDpi_ = 96;
   /** @private */
   this.resizable_ = true;
-  this.defineEvents(Object.keys(remoting.ClientPlugin.HostDesktop.Events));
+  this.defineEvents(base.values(remoting.HostDesktop.Events));
 };
 base.extend(remoting.MockClientPlugin.HostDesktop, base.EventSourceImpl);
 
@@ -189,26 +190,18 @@
remoting.MockClientPlugin.HostDesktop.prototype.isResizable = function() {
 };
 
 /**
- * @return {boolean}
- * @override
- */
-remoting.MockClientPlugin.HostDesktop.prototype.hasResizeRateLimit =
-    function() {
-      return false;
-    };
-
-/**
  * @param {number} width
  * @param {number} height
- * @param {number} dpi
+ * @param {number} deviceScale
  * @override
  */
-remoting.MockClientPlugin.HostDesktop.prototype.resize = function(
-    width, height, device_scale) {
+remoting.MockClientPlugin.HostDesktop.prototype.resize =
+    function(width, height, deviceScale) {
   this.width_ = width;
   this.height_ = height;
-  this.xDpi_ = this.yDpi_ = device_scale;
-  this.raiseEvent(remoting.ClientPlugin.HostDesktop.Events.sizeChanged);
+  this.xDpi_ = this.yDpi_ = Math.floor(deviceScale * 96);
+  this.raiseEvent(remoting.HostDesktop.Events.sizeChanged,
+                  this.getDimensions());
 };
 
 /**

[commit-bot: I haz the power, 2015-02-19 23:56:02]

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

[kelvinp, 2015-02-20 00:15:21]

The CQ bit was checked by kelvinp@chromium.org

[commit-bot: I haz the power, 2015-02-20 00:16:28]

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

[commit-bot: I haz the power, 2015-02-20 00:17:58]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2015-02-20 00:18:28]

Patchset 5 (id:??) landed as
https://crrev.com/87368c7af39e47c717e56e5436b510b1cb3a5eaa
Cr-Commit-Position: refs/heads/master@{#317187}