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

Issue 23498025: Catch ERROR_INVALID_WINDOW_HANDLE in the quit chrome script. (Closed)

Created:
7 years, 3 months ago by sukolsak
Modified:
7 years, 3 months ago
CC:
chromium-reviews
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Catch ERROR_INVALID_WINDOW_HANDLE in the quit chrome script. In the mini_installer test framework, the script quit_chrome.py quits Chrome by sending a WM_CLOSE message to each window of Chrome. Once we send the message to a window, it's possible that the handles to other windows may become invalid (see buildbot error message http://goo.gl/16VmkX). Fix this by catching ERROR_INVALID_WINDOW_HANDLE. NOTRY=True BUG=264859 TEST= 1) Uninstall Chrome (if it's installed.) 2) Build Chrome with Release mode and make sure that mini_installer.exe is created. 3) Go to src\chrome\test\mini_installer 4) Edit quit_chrome.py. Add line "hwnd = 12345" before the line "win32gui.PostMessage(hwnd, win32con.WM_CLOSE, 0, 0)". Add line "print 'ok'" after the line "raise". Then save. 5) Run "python test_installer.py config\config.config --build-dir=<build-dir> --target=Release" where <build-dir> is the path to main build directory (the parent of the Release directory). It should print "ok" repeatedly. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=222098

Patch Set 1 #

Total comments: 3

Patch Set 2 : Address grt's comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -1 line) Patch
M chrome/test/mini_installer/quit_chrome.py View 1 2 chunks +8 lines, -1 line 0 comments Download

Messages

Total messages: 8 (0 generated)
sukolsak
Could you please take a look at this?
7 years, 3 months ago (2013-09-09 18:03:04 UTC) #1
grt (UTC plus 2)
lgtm https://codereview.chromium.org/23498025/diff/1/chrome/test/mini_installer/quit_chrome.py File chrome/test/mini_installer/quit_chrome.py (right): https://codereview.chromium.org/23498025/diff/1/chrome/test/mini_installer/quit_chrome.py#newcode44 chrome/test/mini_installer/quit_chrome.py:44: pass is this more idomatic than: # It's ...
7 years, 3 months ago (2013-09-09 18:53:59 UTC) #2
gab
lgtm https://chromiumcodereview.appspot.com/23498025/diff/1/chrome/test/mini_installer/quit_chrome.py File chrome/test/mini_installer/quit_chrome.py (right): https://chromiumcodereview.appspot.com/23498025/diff/1/chrome/test/mini_installer/quit_chrome.py#newcode44 chrome/test/mini_installer/quit_chrome.py:44: pass On 2013/09/09 18:53:59, grt wrote: > is ...
7 years, 3 months ago (2013-09-09 19:29:05 UTC) #3
robertshield
LGTM w/grtgab's comment addressed
7 years, 3 months ago (2013-09-09 20:07:11 UTC) #4
Mathieu
lgtm
7 years, 3 months ago (2013-09-09 20:14:41 UTC) #5
sukolsak
Thank you. https://codereview.chromium.org/23498025/diff/1/chrome/test/mini_installer/quit_chrome.py File chrome/test/mini_installer/quit_chrome.py (right): https://codereview.chromium.org/23498025/diff/1/chrome/test/mini_installer/quit_chrome.py#newcode44 chrome/test/mini_installer/quit_chrome.py:44: pass On 2013/09/09 18:53:59, grt wrote: > ...
7 years, 3 months ago (2013-09-09 21:26:41 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sukolsak@chromium.org/23498025/11001
7 years, 3 months ago (2013-09-09 21:28:32 UTC) #7
commit-bot: I haz the power
7 years, 3 months ago (2013-09-09 21:30:53 UTC) #8
Message was sent while issue was closed.
Change committed as 222098

Powered by Google App Engine
This is Rietveld 408576698