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

Issue 9567033: Cleanup error handling in the client plugin. (Closed)

Created:
8 years, 9 months ago by Sergey Ulanov
Modified:
8 years, 9 months ago
Reviewers:
simonmorris, Jamie
CC:
chromium-reviews, jamiewalch+watch_chromium.org, dcaiafa+watch_chromium.org, simonmorris+watch_chromium.org, hclam+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, alexeypa+watch_chromium.org, sergeyu+watch_chromium.org
Visibility:
Public.

Description

Cleanup error handling in the client plugin. - Added new ErrorCode enum to pass error codes everywhere except between the webapp and the plugin. - Signaling timeout when connection now is interpreted as disconnect instead of error condition (see bug 112739). - Fixed webapp to properly handle unknown error codes. BUG=112739 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124606

Patch Set 1 #

Total comments: 4

Patch Set 2 : - #

Patch Set 3 : - #

Patch Set 4 : - #

Unified diffs Side-by-side diffs Delta from patch set Stats (+116 lines, -98 lines) Patch
M remoting/client/chromoting_client.h View 1 chunk +1 line, -1 line 0 comments Download
M remoting/client/chromoting_client.cc View 1 chunk +1 line, -1 line 0 comments Download
M remoting/client/chromoting_view.h View 1 chunk +1 line, -1 line 0 comments Download
M remoting/client/plugin/pepper_view.h View 1 chunk +1 line, -1 line 0 comments Download
M remoting/client/plugin/pepper_view.cc View 1 2 3 2 chunks +19 lines, -7 lines 0 comments Download
M remoting/host/client_session.h View 1 chunk +1 line, -1 line 0 comments Download
M remoting/host/client_session.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M remoting/protocol/connection_to_client.h View 1 chunk +1 line, -1 line 0 comments Download
M remoting/protocol/connection_to_client_unittest.cc View 1 2 3 1 chunk +2 lines, -3 lines 0 comments Download
M remoting/protocol/connection_to_host.h View 4 chunks +5 lines, -12 lines 0 comments Download
M remoting/protocol/connection_to_host.cc View 1 2 3 6 chunks +18 lines, -22 lines 0 comments Download
A remoting/protocol/errors.h View 1 2 3 1 chunk +26 lines, -0 lines 0 comments Download
M remoting/protocol/fake_session.h View 3 chunks +3 lines, -3 lines 0 comments Download
M remoting/protocol/fake_session.cc View 1 chunk +1 line, -1 line 0 comments Download
M remoting/protocol/jingle_session.h View 3 chunks +3 lines, -3 lines 0 comments Download
M remoting/protocol/jingle_session.cc View 1 2 3 7 chunks +13 lines, -21 lines 0 comments Download
M remoting/protocol/protocol_mock_objects.h View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M remoting/protocol/session.h View 3 chunks +2 lines, -12 lines 0 comments Download
M remoting/remoting.gyp View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M remoting/webapp/client_plugin_async.js View 1 2 3 1 chunk +10 lines, -4 lines 0 comments Download
M remoting/webapp/client_session.js View 1 chunk +1 line, -0 lines 0 comments Download
M remoting/webapp/server_log_entry.js View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Sergey Ulanov
8 years, 9 months ago (2012-03-02 00:06:40 UTC) #1
simonmorris
Drive-by. http://codereview.chromium.org/9567033/diff/1/remoting/webapp/client_session.js File remoting/webapp/client_session.js (right): http://codereview.chromium.org/9567033/diff/1/remoting/webapp/client_session.js#newcode86 remoting/webapp/client_session.js:86: UNKNOWN: -1, Also update the switch(connectionError) statement in ...
8 years, 9 months ago (2012-03-02 00:23:42 UTC) #2
Sergey Ulanov
http://codereview.chromium.org/9567033/diff/1/remoting/webapp/client_session.js File remoting/webapp/client_session.js (right): http://codereview.chromium.org/9567033/diff/1/remoting/webapp/client_session.js#newcode86 remoting/webapp/client_session.js:86: UNKNOWN: -1, On 2012/03/02 00:23:42, simonmorris wrote: > Also ...
8 years, 9 months ago (2012-03-02 00:38:13 UTC) #3
Jamie
lgtm http://codereview.chromium.org/9567033/diff/1/remoting/protocol/connection_to_host.cc File remoting/protocol/connection_to_host.cc (right): http://codereview.chromium.org/9567033/diff/1/remoting/protocol/connection_to_host.cc#newcode187 remoting/protocol/connection_to_host.cc:187: // are finished moving the client plugin to ...
8 years, 9 months ago (2012-03-02 00:48:12 UTC) #4
Sergey Ulanov
http://codereview.chromium.org/9567033/diff/1/remoting/protocol/connection_to_host.cc File remoting/protocol/connection_to_host.cc (right): http://codereview.chromium.org/9567033/diff/1/remoting/protocol/connection_to_host.cc#newcode187 remoting/protocol/connection_to_host.cc:187: // are finished moving the client plugin to NaCl. ...
8 years, 9 months ago (2012-03-02 01:04:54 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyu@chromium.org/9567033/7023
8 years, 9 months ago (2012-03-02 02:33:33 UTC) #6
commit-bot: I haz the power
8 years, 9 months ago (2012-03-02 05:43:00 UTC) #7
Change committed as 124606

Powered by Google App Engine
This is Rietveld 408576698