[Chromoting] Convert VideoFrameRecorder into a proper protocol extension.

[Chromoting] Convert VideoFrameRecorder into a proper protocol extension.

BUG=465878
Committed: https://crrev.com/8b7bac0f6011d1b0f615336037763a36e6943799
Cr-Commit-Position: refs/heads/master@{#321903}

[garykac, 2015-03-20 22:17:41]

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

[Wez, 2015-03-20 23:53:39]

wez@chromium.org changed reviewers:
+ wez@chromium.org

[Wez, 2015-03-20 23:53:40]

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_connected_view.js (left):

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_connected_view.js:299:
remoting.DesktopConnectedView.prototype.handleExtensionMessage =
Not sure where this was even being called from under the new refactoring..?

[garykac, 2015-03-21 00:06:00]

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_connected_view.js (left):

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_connected_view.js:299:
remoting.DesktopConnectedView.prototype.handleExtensionMessage =
On 2015/03/20 23:53:40, Wez wrote:
> Not sure where this was even being called from under the new refactoring..?

It wasn't. The call was accidentally removed in a previous cl.

See the last change in:
https://codereview.chromium.org/1016623002/diff/80001/remoting/webapp/crd/js/...
to see the previous call site.

[kelvinp, 2015-03-21 17:36:09]

lgtm with Nits.

Not related to this CL, but I am wondering who should own the protocol
extension.  Should the session_connector be responsible for cleaning up the
extension when a session finishes?

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_connected_view.js (right):

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_connected_view.js:279: if
(!this.videoFrameRecorder_) {
Indent

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/video_frame_recorder.js (right):

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/video_frame_recorder.js:63: if (type !=
remoting.VideoFrameRecorder.EXTENSION_TYPE) {
Nit: Assert instead.

[garykac, 2015-03-23 22:51:04]

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/desktop_connected_view.js (right):

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/desktop_connected_view.js:279: if
(!this.videoFrameRecorder_) {
On 2015/03/21 17:36:09, kelvinp wrote:
> Indent

Done.

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/video_frame_recorder.js (right):

https://codereview.chromium.org/1028803002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/video_frame_recorder.js:63: if (type !=
remoting.VideoFrameRecorder.EXTENSION_TYPE) {
On 2015/03/21 17:36:09, kelvinp wrote:
> Nit: Assert instead.

Done.

[garykac, 2015-03-23 22:51:11]

The CQ bit was checked by garykac@chromium.org

[garykac, 2015-03-23 22:51:12]

The patchset sent to the CQ was uploaded after l-g-t-m from kelvinp@chromium.org
Link to the patchset: https://codereview.chromium.org/1028803002/#ps40001
(title: "Fix nits")

[commit-bot: I haz the power, 2015-03-23 22:52:24]

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

[commit-bot: I haz the power, 2015-03-24 00:08:09]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-24 00:08:48]

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