Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1607143002:
Fixing the Remote Desktop Browser Test (Me2Me connection test) Failure (Closed)
DescriptionFixing the Remote Desktop BrowserTest Failure
Merge branch 'master' of https://chromium.googlesource.com/chromium/src
Merge branch 'master' of https://chromium.googlesource.com/chromium/src
The following code change introduced an issue:
https://codereview.chromium.org/1554313002
There a couple of issues causing the problem.
1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout.
2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the result from Refreshing the Chromoting Window.
From:
bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()");
To:
ExecuteScript("remoting.hostList.refreshAndDisplay()");
BUG=577465
Committed: https://crrev.com/1c4a0ed002dc2902473d1515c390ffe081890b12
Cr-Commit-Position: refs/heads/master@{#370798}
Patch Set 1 #Patch Set 2 #
Total comments: 2
Patch Set 3 #Messages
Total messages: 19 (9 generated)
alog@google.com changed reviewers: + joedow@chromium.org
Can you update the CL description? It mentions fixing the owners file but that doesn't seem to be what is happening here. Also, Can you add some details on why you removed the code that you did and what the failure was that you are fixing. It would be good to give some context around the change to the reviewer. Thanks! Joe
Description was changed from ========== Fixing the Remote Desktop BrowserTest Failre Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src Updating the Chromoting Browser Tests Owners File BUG= ========== to ========== Fixing the Remote Desktop BrowserTest Failre Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 The issue is that, the following line is failing and there by resulting in Chromoting waterfall failure. As a quick patch, it is removed in this CL. bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); BUG=577465 ==========
lgtm. Can you address the comment and update the CL description to match the change. https://codereview.chromium.org/1607143002/diff/20001/chrome/test/remoting/re... File chrome/test/remoting/remote_desktop_browsertest.cc (right): https://codereview.chromium.org/1607143002/diff/20001/chrome/test/remoting/re... chrome/test/remoting/remote_desktop_browsertest.cc:590: ConditionalTimeoutWaiter hostOnlineWaiter( You may want to add a comment about what this code is doing (3 attempts at 20 seconds each) since it caused some confusion earlier.
Description was changed from ========== Fixing the Remote Desktop BrowserTest Failre Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 The issue is that, the following line is failing and there by resulting in Chromoting waterfall failure. As a quick patch, it is removed in this CL. bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); BUG=577465 ========== to ========== Fixing the Remote Desktop BrowserTest Failure Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 There a couple of issues causing the problem. 1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout. 2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the Refresh Result. From: bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); To: ExecuteScript("remoting.hostList.refreshAndDisplay()"); BUG=577465 ==========
Description was changed from ========== Fixing the Remote Desktop BrowserTest Failure Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 There a couple of issues causing the problem. 1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout. 2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the Refresh Result. From: bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); To: ExecuteScript("remoting.hostList.refreshAndDisplay()"); BUG=577465 ========== to ========== Fixing the Remote Desktop BrowserTest Failure Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 There a couple of issues causing the problem. 1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout. 2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the Refresh Result. From: bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); To: ExecuteScript("remoting.hostList.refreshAndDisplay()"); BUG=577465 ==========
Description was changed from ========== Fixing the Remote Desktop BrowserTest Failure Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 There a couple of issues causing the problem. 1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout. 2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the Refresh Result. From: bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); To: ExecuteScript("remoting.hostList.refreshAndDisplay()"); BUG=577465 ========== to ========== Fixing the Remote Desktop BrowserTest Failure Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 There a couple of issues causing the problem. 1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout. 2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the result from Refreshing the Chromoting Window. From: bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); To: ExecuteScript("remoting.hostList.refreshAndDisplay()"); BUG=577465 ==========
There a couple of issues causing the current Chromoting Waterfall failure.
1. One is the conditional waiter for HostOnline (in
remote_desktop_browsertest.cc line 590) having short timeout. This is addressed
by increasing the timeout for each try.
2. Second issue is that, the following line times out and there by resulting in
Chromoting waterfall failure. This is changed to ExecuteScript, since we don't
need to validate the result from Refreshing the Chromoting Window.
From:
bool refresh_host_list =
ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()");
To:
ExecuteScript("remoting.hostList.refreshAndDisplay()");
https://codereview.chromium.org/1607143002/diff/20001/chrome/test/remoting/re...
File chrome/test/remoting/remote_desktop_browsertest.cc (right):
https://codereview.chromium.org/1607143002/diff/20001/chrome/test/remoting/re...
chrome/test/remoting/remote_desktop_browsertest.cc:590: ConditionalTimeoutWaiter
hostOnlineWaiter(
On 2016/01/20 22:23:06, joedow wrote:
> You may want to add a comment about what this code is doing (3 attempts at 20
> seconds each) since it caused some confusion earlier.
Done.
The CQ bit was checked by alog@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from joedow@chromium.org Link to the patchset: https://codereview.chromium.org/1607143002/#ps40001 (title: "")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1607143002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1607143002/40001
Message was sent while issue was closed.
Description was changed from ========== Fixing the Remote Desktop BrowserTest Failure Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 There a couple of issues causing the problem. 1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout. 2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the result from Refreshing the Chromoting Window. From: bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); To: ExecuteScript("remoting.hostList.refreshAndDisplay()"); BUG=577465 ========== to ========== Fixing the Remote Desktop BrowserTest Failure Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 There a couple of issues causing the problem. 1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout. 2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the result from Refreshing the Chromoting Window. From: bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); To: ExecuteScript("remoting.hostList.refreshAndDisplay()"); BUG=577465 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Fixing the Remote Desktop BrowserTest Failure Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 There a couple of issues causing the problem. 1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout. 2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the result from Refreshing the Chromoting Window. From: bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); To: ExecuteScript("remoting.hostList.refreshAndDisplay()"); BUG=577465 ========== to ========== Fixing the Remote Desktop BrowserTest Failure Merge branch 'master' of https://chromium.googlesource.com/chromium/src Merge branch 'master' of https://chromium.googlesource.com/chromium/src The following code change introduced an issue: https://codereview.chromium.org/1554313002 There a couple of issues causing the problem. 1. One is the conditional waiter for HostOnline having short timeout. This is addressed by increasing the timeout. 2. Second issue is that, the following line times out and there by resulting in Chromoting waterfall failure. This is changed to ExecuteScript, since we don't need to validate the result from Refreshing the Chromoting Window. From: bool refresh_host_list = ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()"); To: ExecuteScript("remoting.hostList.refreshAndDisplay()"); BUG=577465 Committed: https://crrev.com/1c4a0ed002dc2902473d1515c390ffe081890b12 Cr-Commit-Position: refs/heads/master@{#370798} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/1c4a0ed002dc2902473d1515c390ffe081890b12 Cr-Commit-Position: refs/heads/master@{#370798} |
