diff options
| author | brucedawson <brucedawson@chromium.org> | 2015-09-25 12:10:44 -0700 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2015-09-25 19:11:17 +0000 |
| commit | a1e13db86655aa7737e141111881454219a23350 (patch) | |
| tree | 9d250d85523c5d5f4234a108151182084077db51 /base/win | |
| parent | c0ee40f9504c8044177e71b1b425c80829b00e73 (diff) | |
| download | chromium_src-a1e13db86655aa7737e141111881454219a23350.zip chromium_src-a1e13db86655aa7737e141111881454219a23350.tar.gz chromium_src-a1e13db86655aa7737e141111881454219a23350.tar.bz2 | |
Don't wait for ETW controller.Stop() on Windows 7
On Windows 7 the ControlTrace() function with EVENT_TRACE_CONTROL_STOP,
called by EtwTraceController::Stop, does not call the callback function.
This means that calling WaitForCallback() will hang on Windows 7, and
enable_level() and enable_flags() will never return zero.
This was hidden initially because there was no WaitForCallback after
the Register call so that the test usually passed on Windows 8, but had
a race condition.
On Windows 7 the test usually didn't run due to E_ACCESSDENIED due to
the user not being in the Performance Log Users group which is why it
didn't fail there. The test is now hanging sometimes on Windows 7.
Since this change should fix the test it also reenables it.
This change also avoids stopping tracing in the EtwTraceController
destructor if tracing is not running, to avoid confusing failures.
R=danakj@chromium.org
BUG=525297
Review URL: https://codereview.chromium.org/1358763003
Cr-Commit-Position: refs/heads/master@{#350878}
Diffstat (limited to 'base/win')
| -rw-r--r-- | base/win/event_trace_controller.cc | 3 | ||||
| -rw-r--r-- | base/win/event_trace_controller_unittest.cc | 16 |
2 files changed, 12 insertions, 7 deletions
diff --git a/base/win/event_trace_controller.cc b/base/win/event_trace_controller.cc index 9a35a6b..ff392a3 100644 --- a/base/win/event_trace_controller.cc +++ b/base/win/event_trace_controller.cc @@ -46,7 +46,8 @@ EtwTraceController::EtwTraceController() : session_(NULL) { } EtwTraceController::~EtwTraceController() { - Stop(NULL); + if (session_) + Stop(NULL); } HRESULT EtwTraceController::Start(const wchar_t* session_name, diff --git a/base/win/event_trace_controller_unittest.cc b/base/win/event_trace_controller_unittest.cc index 625a92f..3173275 100644 --- a/base/win/event_trace_controller_unittest.cc +++ b/base/win/event_trace_controller_unittest.cc @@ -17,6 +17,7 @@ #include "base/win/event_trace_controller.h" #include "base/win/event_trace_provider.h" #include "base/win/scoped_handle.h" +#include "base/win/windows_version.h" #include "testing/gtest/include/gtest/gtest.h" namespace base { @@ -179,8 +180,7 @@ TEST_F(EtwTraceControllerTest, StartFileSession) { base::DeleteFile(temp, false); } -// Disable until bug 525297 is fixed (hangs on Windows 7?) -TEST_F(EtwTraceControllerTest, DISABLED_EnableDisable) { +TEST_F(EtwTraceControllerTest, EnableDisable) { TestingProvider provider(test_provider_); EXPECT_EQ(ERROR_SUCCESS, provider.Register()); @@ -227,11 +227,15 @@ TEST_F(EtwTraceControllerTest, DISABLED_EnableDisable) { EXPECT_HRESULT_SUCCEEDED(controller.Stop(NULL)); - provider.WaitForCallback(); + // Windows 7 does not call the callback when Stop() is called so we + // can't wait, and enable_level and enable_flags are not zeroed. + if (base::win::GetVersion() >= VERSION_WIN8) { + provider.WaitForCallback(); - // Session should have wound down. - EXPECT_EQ(0, provider.enable_level()); - EXPECT_EQ(0, provider.enable_flags()); + // Session should have wound down. + EXPECT_EQ(0, provider.enable_level()); + EXPECT_EQ(0, provider.enable_flags()); + } } } // namespace win |