Revert 62766 - Fix event tracing for windows test flakyness.

Assert on non-null properties to the static members of the trace controller.
Fix usage of same.
Add a PostEventsDisabled method to trace producer to which allows for a non-racy enable/disable test.

BUG=52388
BUG=59328
TEST=Unittests in the change.

Review URL: http://codereview.chromium.org/3800006

TBR=siggi@chromium.org
Review URL: http://codereview.chromium.org/3815008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@62767 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 13 insertions, 40 deletions

diff --git a/base/event_trace_consumer_win_unittest.cc b/base/event_trace_consumer_win_unittest.cc
index c964412..6c0a740 100644
--- a/base/event_trace_consumer_win_unittest.cc
+++ b/base/event_trace_consumer_win_unittest.cc
@@ -72,8 +72,7 @@ const wchar_t* const kTestSessionName = L"TestLogSession";
 class EtwTraceConsumerBaseTest: public testing::Test {
  public:
   virtual void SetUp() {
-    EtwTraceProperties ignore;
-    EtwTraceController::Stop(kTestSessionName, &ignore);
+    EtwTraceController::Stop(kTestSessionName, NULL);
   }
 };
 
@@ -246,16 +245,14 @@ class EtwTraceConsumerDataTest: public testing::Test {
   }
 
   virtual void SetUp() {
-    EtwTraceProperties prop;
-    EtwTraceController::Stop(kTestSessionName, &prop);
+    EtwTraceController::Stop(kTestSessionName, NULL);
     // Construct a temp file name.
     ASSERT_TRUE(file_util::CreateTemporaryFile(&temp_file_));
   }
 
   virtual void TearDown() {
     EXPECT_TRUE(file_util::Delete(temp_file_, false));
-    EtwTraceProperties ignore;
-    EtwTraceController::Stop(kTestSessionName, &ignore);
+    EtwTraceController::Stop(kTestSessionName, NULL);
   }
 
   HRESULT LogEventToTempSession(PEVENT_TRACE_HEADER header) {
diff --git a/base/event_trace_controller_win.cc b/base/event_trace_controller_win.cc
index c46164b..cb15150 100644
--- a/base/event_trace_controller_win.cc
+++ b/base/event_trace_controller_win.cc
@@ -97,7 +97,6 @@ HRESULT EtwTraceController::Flush(EtwTraceProperties* properties) {
 
 HRESULT EtwTraceController::Start(const wchar_t* session_name,
     EtwTraceProperties* properties, TRACEHANDLE* session_handle) {
-  DCHECK(properties != NULL);
   ULONG err = ::StartTrace(session_handle, session_name, properties->get());
   return HRESULT_FROM_WIN32(err);
 }
@@ -111,7 +110,6 @@ HRESULT EtwTraceController::Query(const wchar_t* session_name,
 
 HRESULT EtwTraceController::Update(const wchar_t* session_name,
     EtwTraceProperties* properties) {
-  DCHECK(properties != NULL);
   ULONG err = ::ControlTrace(NULL, session_name, properties->get(),
                              EVENT_TRACE_CONTROL_UPDATE);
   return HRESULT_FROM_WIN32(err);
@@ -119,7 +117,6 @@ HRESULT EtwTraceController::Update(const wchar_t* session_name,
 
 HRESULT EtwTraceController::Stop(const wchar_t* session_name,
     EtwTraceProperties* properties) {
-  DCHECK(properties != NULL);
   ULONG err = ::ControlTrace(NULL, session_name, properties->get(),
                              EVENT_TRACE_CONTROL_STOP);
   return HRESULT_FROM_WIN32(err);
@@ -127,7 +124,6 @@ HRESULT EtwTraceController::Stop(const wchar_t* session_name,
 
 HRESULT EtwTraceController::Flush(const wchar_t* session_name,
     EtwTraceProperties* properties) {
-  DCHECK(properties != NULL);
   ULONG err = ::ControlTrace(NULL, session_name, properties->get(),
                              EVENT_TRACE_CONTROL_FLUSH);
   return HRESULT_FROM_WIN32(err);
diff --git a/base/event_trace_controller_win_unittest.cc b/base/event_trace_controller_win_unittest.cc
index 5777e58..55222a6 100644
--- a/base/event_trace_controller_win_unittest.cc
+++ b/base/event_trace_controller_win_unittest.cc
@@ -42,7 +42,7 @@ class TestingProvider: public EtwTraceProvider {
   virtual void OnEventsEnabled() {
     ::SetEvent(callback_event_.Get());
   }
-  virtual void PostEventsDisabled() {
+  virtual void OnEventsDisabled() {
     ::SetEvent(callback_event_.Get());
   }
 
@@ -55,8 +55,7 @@ class TestingProvider: public EtwTraceProvider {
 
 TEST(EtwTraceTest, Cleanup) {
   // Clean up potential leftover sessions from previous unsuccessful runs.
-  EtwTraceProperties ignore;
-  EtwTraceController::Stop(kTestSessionName, &ignore);
+  EtwTraceController::Stop(kTestSessionName, NULL);
 }
 
 TEST(EtwTracePropertiesTest, Initialization) {
@@ -153,7 +152,8 @@ TEST(EtwTraceControllerTest, StartFileSession) {
   EXPECT_STREQ(L"", controller.session_name());
 }
 
-TEST(EtwTraceControllerTest, EnableDisable) {
+// Flaky, http://crbug.com/59328.
+TEST(EtwTraceControllerTest, FLAKY_EnableDisable) {
   TestingProvider provider(kTestProvider);
 
   EXPECT_EQ(ERROR_SUCCESS, provider.Register());
diff --git a/base/event_trace_provider_win.cc b/base/event_trace_provider_win.cc
index 826a4bf..c7f5001 100644
--- a/base/event_trace_provider_win.cc
+++ b/base/event_trace_provider_win.cc
@@ -48,8 +48,6 @@ ULONG EtwTraceProvider::DisableEvents() {
   enable_flags_ = 0;
   session_handle_ = NULL;
 
-  PostEventsDisabled();
-
   return ERROR_SUCCESS;
 }
 
diff --git a/base/event_trace_provider_win.h b/base/event_trace_provider_win.h
index 5c89d1a..b0526b1 100644
--- a/base/event_trace_provider_win.h
+++ b/base/event_trace_provider_win.h
@@ -126,25 +126,13 @@ class EtwTraceProvider {
   ULONG Log(EVENT_TRACE_HEADER* event);
 
  protected:
-  // Called after events have been enabled, override in subclasses
-  // to set up state or log at the start of a session.
-  // Note: This function may be called ETW's thread and may be racy,
-  //    bring your own locking if needed.
+  // These are called after events have been enabled or disabled.
+  // Override them if you want to do processing at the start or
+  // end of collection.
+  // Note: These may be called ETW's thread and they may be racy.
   virtual void OnEventsEnabled() {}
-
-  // Called just before events are disabled, override in subclasses
-  // to tear down state or log at the end of a session.
-  // Note: This function may be called ETW's thread and may be racy,
-  //    bring your own locking if needed.
   virtual void OnEventsDisabled() {}
 
-  // Called just after events have been disabled, override in subclasses
-  // to tear down state at the end of a session. At this point it's
-  // to late to log anything to the session.
-  // Note: This function may be called ETW's thread and may be racy,
-  //    bring your own locking if needed.
-  virtual void PostEventsDisabled() {}
-
  private:
   ULONG EnableEvents(PVOID buffer);
   ULONG DisableEvents();
diff --git a/base/trace_event_win_unittest.cc b/base/trace_event_win_unittest.cc
index 1335bed..a142af1 100644
--- a/base/trace_event_win_unittest.cc
+++ b/base/trace_event_win_unittest.cc
@@ -79,13 +79,6 @@ class TraceEventTest: public testing::Test {
   void SetUp() {
     bool is_xp = base::win::GetVersion() < base::win::VERSION_VISTA;
 
-    if (is_xp) {
-      // Tear down any dangling session from an earlier failing test.
-      EtwTraceProperties ignore;
-
-      EtwTraceController::Stop(kTestSessionName, &ignore);
-    }
-
     // Resurrect and initialize the TraceLog singleton instance.
     // On Vista and better, we need the provider registered before we
     // start the private, in-proc session, but on XP we need the global
@@ -261,7 +254,8 @@ TEST_F(TraceEventTest, TraceLog) {
   PlayLog();
 }
 
-TEST_F(TraceEventTest, Macros) {
+// Marked flaky per http://crbug.com/52388
+TEST_F(TraceEventTest, FLAKY_Macros) {
   ExpectPlayLog();
 
   // The events should arrive in the same sequence as the expects.