Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(388)

Issue 386853002: Add a Record button to the web-app if the host supports video recording. (Closed)

Created:
6 years, 5 months ago by Wez
Modified:
6 years, 4 months ago
CC:
chromium-reviews, chromoting-reviews_chromium.org
Project:
chromium
Visibility:
Public.

Description

Add a Record button to the web-app if the host supports video recording. Video recording allows sequences of frames to be recorded and delivered to the client for performance-evaluation purposes. The client can save recorded sequences out to files on disk. Since live playback is not a requirement, performance of the implementation is not a priority. See crrev.com/372943002 for the host side of this. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290721

Patch Set 1 #

Patch Set 2 : Rebase #

Total comments: 24

Patch Set 3 : Move implementation out to remoting.VideoFrameRecorder. #

Total comments: 2

Patch Set 4 : Move code out into OptionsMenu and VideoFrameRecorder #

Total comments: 7

Patch Set 5 : Add missing isRecording and some more comments #

Patch Set 6 : Fix almost all type annotations #

Total comments: 1

Patch Set 7 : Suppress type error for new Blob() #

Patch Set 8 : function -> var #

Unified diffs Side-by-side diffs Delta from patch set Stats (+257 lines, -4 lines) Patch
M remoting/remoting_webapp_files.gypi View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M remoting/resources/remoting_strings.grd View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M remoting/webapp/client_screen.js View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M remoting/webapp/client_session.js View 1 2 3 4 4 chunks +49 lines, -0 lines 0 comments Download
M remoting/webapp/html/window_frame.html View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M remoting/webapp/js_proto/chrome_proto.js View 1 2 3 4 5 2 chunks +14 lines, -1 line 0 comments Download
M remoting/webapp/manifest.json.jinja2 View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M remoting/webapp/options_menu.js View 1 2 3 4 5 4 chunks +24 lines, -1 line 0 comments Download
M remoting/webapp/toolbar.js View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
A remoting/webapp/video_frame_recorder.js View 1 2 3 4 5 6 7 1 chunk +152 lines, -0 lines 0 comments Download
M remoting/webapp/window_frame.js View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 15 (0 generated)
Wez
This is a quick hack of a recording button into the Chromoting client for the ...
6 years, 4 months ago (2014-08-01 00:03:24 UTC) #1
Jamie
https://codereview.chromium.org/386853002/diff/20001/remoting/webapp/client_screen.js File remoting/webapp/client_screen.js (right): https://codereview.chromium.org/386853002/diff/20001/remoting/webapp/client_screen.js#newcode364 remoting/webapp/client_screen.js:364: remoting.clientSession.handleVideoRecorderMessage(type, data)) { Since handleVideoRecorderMessage has side-effects, I think ...
6 years, 4 months ago (2014-08-01 01:28:37 UTC) #2
kelvinp
https://codereview.chromium.org/386853002/diff/40001/remoting/webapp/client_session.js File remoting/webapp/client_session.js (right): https://codereview.chromium.org/386853002/diff/40001/remoting/webapp/client_session.js#newcode1091 remoting/webapp/client_session.js:1091: this.videoFrameRecorder_ = new remoting.VideoFrameRecorder(); This change looks pretty cool. ...
6 years, 4 months ago (2014-08-15 23:37:31 UTC) #3
Sergey Ulanov
https://codereview.chromium.org/386853002/diff/40001/remoting/webapp/client_session.js File remoting/webapp/client_session.js (right): https://codereview.chromium.org/386853002/diff/40001/remoting/webapp/client_session.js#newcode1091 remoting/webapp/client_session.js:1091: this.videoFrameRecorder_ = new remoting.VideoFrameRecorder(); On 2014/08/15 23:37:31, kelvinp wrote: ...
6 years, 4 months ago (2014-08-16 00:16:58 UTC) #4
Wez
https://codereview.chromium.org/386853002/diff/20001/remoting/webapp/client_screen.js File remoting/webapp/client_screen.js (right): https://codereview.chromium.org/386853002/diff/20001/remoting/webapp/client_screen.js#newcode364 remoting/webapp/client_screen.js:364: remoting.clientSession.handleVideoRecorderMessage(type, data)) { On 2014/08/01 01:28:36, Jamie wrote: > ...
6 years, 4 months ago (2014-08-19 00:15:53 UTC) #5
Jamie
LGTM once it compiles, but if you want to me take another look at that ...
6 years, 4 months ago (2014-08-19 00:48:57 UTC) #6
Wez
https://codereview.chromium.org/386853002/diff/20001/remoting/resources/remoting_strings.grd File remoting/resources/remoting_strings.grd (right): https://codereview.chromium.org/386853002/diff/20001/remoting/resources/remoting_strings.grd#newcode710 remoting/resources/remoting_strings.grd:710: <message desc="Button for starting and stopping video frame recording, ...
6 years, 4 months ago (2014-08-19 21:19:00 UTC) #7
Wez
The CQ bit was checked by wez@chromium.org
6 years, 4 months ago (2014-08-19 21:37:41 UTC) #8
Wez
FYI
6 years, 4 months ago (2014-08-19 21:37:48 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wez@chromium.org/386853002/120001
6 years, 4 months ago (2014-08-19 21:39:00 UTC) #10
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_clang_dbg on tryserver.chromium.linux ...
6 years, 4 months ago (2014-08-19 23:27:06 UTC) #11
Wez
The CQ bit was checked by wez@chromium.org
6 years, 4 months ago (2014-08-19 23:46:37 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wez@chromium.org/386853002/140001
6 years, 4 months ago (2014-08-19 23:46:47 UTC) #13
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_clang_dbg on tryserver.chromium.linux ...
6 years, 4 months ago (2014-08-20 01:01:40 UTC) #14
commit-bot: I haz the power
6 years, 4 months ago (2014-08-20 01:12:28 UTC) #15
Message was sent while issue was closed.
Committed patchset #8 (140001) as 290721

Powered by Google App Engine
This is Rietveld 408576698