Clean up client state callback.

Clean up client state callback.

BUG=None
TEST=Manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=111036

[Jamie, 2011-11-16 00:17:12]

PTAL

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
File remoting/webapp/me2mom/client_session.js (left):

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
remoting/webapp/me2mom/client_session.js:340: window.innerHeight);
Unrelated to this CL; it was just bugging me.

[Wez, 2011-11-16 21:58:26]

lgtm

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
File remoting/webapp/me2mom/client_screen.js (right):

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
remoting/webapp/me2mom/client_screen.js:182: * @param {number} newState The
previous state of the plugin.
typo: The _new_ state of the plugin?

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
File remoting/webapp/me2mom/client_session.js (right):

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
remoting/webapp/me2mom/client_session.js:45: // Note that the (positive) values
in both of these enums are copied directly
Why is positive in brackets?  Are the negative values copied or not?

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/viewer_p...
File remoting/webapp/me2mom/viewer_plugin_proto.js (right):

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/viewer_p...
remoting/webapp/me2mom/viewer_plugin_proto.js:36:
remoting.ViewerPlugin.prototype.connectionInfoUpdate;
Did we not need this before because of some default for function():void?

[Sergey Ulanov, 2011-11-16 23:08:12]

http://codereview.chromium.org/8573024/diff/7/remoting/client/plugin/chromoti...
File remoting/client/plugin/chromoting_scriptable_object.cc (right):

http://codereview.chromium.org/8573024/diff/7/remoting/client/plugin/chromoti...
remoting/client/plugin/chromoting_scriptable_object.cc:322: cb.Call(Var(),
Var(status), Var(error), &exception);
Will this work with an older version of the webapp that didn't have these
parameters?

http://codereview.chromium.org/8573024/diff/7/remoting/client/plugin/chromoti...
File remoting/client/plugin/chromoting_scriptable_object.h (right):

http://codereview.chromium.org/8573024/diff/7/remoting/client/plugin/chromoti...
remoting/client/plugin/chromoting_scriptable_object.h:24: //   const unsigned
short STATUS_UNKNOWN;
Since these values are not used anymore can we remove them from the interface?
Same question regarding status and error properties.
We probably want to keep them for backward compatibility between the webapp and
the plugin. Maybe add TODO to remove them in the future?

[Jamie, 2011-11-17 22:06:17]

http://codereview.chromium.org/8573024/diff/7/remoting/client/plugin/chromoti...
File remoting/client/plugin/chromoting_scriptable_object.cc (right):

http://codereview.chromium.org/8573024/diff/7/remoting/client/plugin/chromoti...
remoting/client/plugin/chromoting_scriptable_object.cc:322: cb.Call(Var(),
Var(status), Var(error), &exception);
On 2011/11/16 23:08:12, sergeyu wrote:
> Will this work with an older version of the webapp that didn't have these
> parameters?

Yes, I think so. Certainly the converse is true--if the caller is old and
doesn't provide these parameters then the function is still invoked; we test for
that case explicitly.

http://codereview.chromium.org/8573024/diff/7/remoting/client/plugin/chromoti...
File remoting/client/plugin/chromoting_scriptable_object.h (right):

http://codereview.chromium.org/8573024/diff/7/remoting/client/plugin/chromoti...
remoting/client/plugin/chromoting_scriptable_object.h:24: //   const unsigned
short STATUS_UNKNOWN;
On 2011/11/16 23:08:12, sergeyu wrote:
> Since these values are not used anymore can we remove them from the interface?
> Same question regarding status and error properties.
> We probably want to keep them for backward compatibility between the webapp
and
> the plugin. Maybe add TODO to remove them in the future?

Done.

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
File remoting/webapp/me2mom/client_screen.js (right):

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
remoting/webapp/me2mom/client_screen.js:182: * @param {number} newState The
previous state of the plugin.
On 2011/11/16 21:58:26, Wez wrote:
> typo: The _new_ state of the plugin?

Done.

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
File remoting/webapp/me2mom/client_session.js (right):

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/client_s...
remoting/webapp/me2mom/client_session.js:45: // Note that the (positive) values
in both of these enums are copied directly
On 2011/11/16 21:58:26, Wez wrote:
> Why is positive in brackets?  Are the negative values copied or not?

No. I've removed the brackets and expanded a bit.

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/viewer_p...
File remoting/webapp/me2mom/viewer_plugin_proto.js (right):

http://codereview.chromium.org/8573024/diff/7/remoting/webapp/me2mom/viewer_p...
remoting/webapp/me2mom/viewer_plugin_proto.js:36:
remoting.ViewerPlugin.prototype.connectionInfoUpdate;
On 2011/11/16 21:58:26, Wez wrote:
> Did we not need this before because of some default for function():void?

It should have been there before, but the compiler didn't complain because it's
fine to write "object.undeclared_field = blah"