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

Issue 3364019: Allow overriding of X error functions (Closed)

Created:
10 years, 3 months ago by DaveMoore
Modified:
9 years, 7 months ago
CC:
chromium-reviews, stuartmorgan+watch_chromium.org, ben+cc_chromium.org, Paweł Hajdan Jr., jam, apatrick_chromium, pam+watch_chromium.org, brettw-cc_chromium.org, darin-cc_chromium.org, davemoore+watch_chromium.org, sky
Base URL:
http://src.chromium.org/git/chromium.git
Visibility:
Public.

Description

This is a second attempt of http://codereview.chromium.org/3175038 It failed the Vista Perf UI tests. This is because those tests close the browser upon an error. And they always get an error when the session is closed in the middle of the test. The new changes are in chrome/browser/automation/testing_automation_provider.cc BUG=50006 TEST=Run chrome under nested window manager using Xephyr (see http://code.google.com/p/chromium/wiki/LayoutTestsLinux) use --enable-logging=stderr --log-level=0 kill xephyr examine log. You should see X IO Error detected followed (not necessarily immediately) by successfully saved /tmp/tx/Default/Preferences successfully saved /tmp/tx/Local State successfully saved /tmp/tx/Local State successfully saved /tmp/tx/Default/Preferences along with no crash. BUG= TEST= Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=59269

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+258 lines, -113 lines) Patch
M app/x11_util.h View 1 chunk +4 lines, -2 lines 0 comments Download
M app/x11_util.cc View 4 chunks +104 lines, -84 lines 0 comments Download
M app/x11_util_internal.h View 2 chunks +12 lines, -2 lines 0 comments Download
M chrome/app/chrome_dll_main.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/app_controller_mac.mm View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/automation/testing_automation_provider.cc View 2 chunks +3 lines, -1 line 1 comment Download
M chrome/browser/browser_list.h View 1 chunk +5 lines, -6 lines 0 comments Download
M chrome/browser/browser_list.cc View 6 chunks +16 lines, -7 lines 0 comments Download
M chrome/browser/browser_main.cc View 2 chunks +5 lines, -0 lines 0 comments Download
A chrome/browser/browser_main_gtk.h View 1 chunk +15 lines, -0 lines 0 comments Download
M chrome/browser/browser_main_gtk.cc View 3 chunks +36 lines, -0 lines 0 comments Download
M chrome/browser/browser_process_impl.cc View 5 chunks +28 lines, -3 lines 0 comments Download
M chrome/browser/browser_shutdown.h View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/browser_shutdown.cc View 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/tab_closeable_state_watcher.cc View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/gtk/bookmark_bar_gtk.cc View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/views/bookmark_bar_view.cc View 2 chunks +5 lines, -2 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/gpu/gpu_thread.cc View 2 chunks +2 lines, -1 line 0 comments Download
M chrome/plugin/plugin_thread.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 2 (0 generated)
DaveMoore
10 years, 3 months ago (2010-09-13 00:50:31 UTC) #1
darin (slow to review)
10 years, 3 months ago (2010-09-13 18:37:07 UTC) #2
http://codereview.chromium.org/3364019/diff/1/7
File chrome/browser/automation/testing_automation_provider.cc (right):

http://codereview.chromium.org/3364019/diff/1/7#newcode423
chrome/browser/automation/testing_automation_provider.cc:423: if
(browser_shutdown::GetShutdownType() == browser_shutdown::NOT_VALID)
LGTM

This might be clearer if we had a helper function named
browser_shutdown::IsShuttingDown()

Powered by Google App Engine
This is Rietveld 408576698