Fix logic reversal bug in TraceMessageFilter

content/browser/trace_message_filter.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/trace_message_filter.h"
 
 #include "content/browser/trace_controller_impl.h"
 #include "content/common/child_process_messages.h"
 
 using content::BrowserMessageFilter;
 using content::BrowserThread;
 using content::TraceControllerImpl;
 
 TraceMessageFilter::TraceMessageFilter() :
     has_child_(false),
     is_awaiting_end_ack_(false),
-    is_awaiting_bpf_ack_(false) {
+    is_awaiting_buffer_percent_full_ack_(false) {
 }
 
 void TraceMessageFilter::OnFilterAdded(IPC::Channel* channel) {
   // Always on IO thread (BrowserMessageFilter guarantee).
   BrowserMessageFilter::OnFilterAdded(channel);
 }
 
 void TraceMessageFilter::OnChannelClosing() {
   // Always on IO thread (BrowserMessageFilter guarantee).
   BrowserMessageFilter::OnChannelClosing();
 
   if (has_child_) {
-    if (is_awaiting_bpf_ack_)
+    if (is_awaiting_end_ack_)
       OnEndTracingAck(std::vector<std::string>());
 
-    if (is_awaiting_end_ack_)
+    if (is_awaiting_buffer_percent_full_ack_)
       OnTraceBufferPercentFullReply(0.0f);
 
     TraceControllerImpl::GetInstance()->RemoveFilter(this);
   }
 }
 
 bool TraceMessageFilter::OnMessageReceived(const IPC::Message& message,
                                            bool* message_was_ok) {
   // Always on IO thread (BrowserMessageFilter guarantee).
   bool handled = true;
@@ skipping 22 matching lines @@
 
 void TraceMessageFilter::SendEndTracing() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!is_awaiting_end_ack_);
   is_awaiting_end_ack_ = true;
   Send(new ChildProcessMsg_EndTracing);
 }
 
 void TraceMessageFilter::SendGetTraceBufferPercentFull() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!is_awaiting_bpf_ack_);
-  is_awaiting_bpf_ack_ = true;
+  DCHECK(!is_awaiting_buffer_percent_full_ack_);
+  is_awaiting_buffer_percent_full_ack_ = true;
   Send(new ChildProcessMsg_GetTraceBufferPercentFull);
 }
 
 TraceMessageFilter::~TraceMessageFilter() {}
 
 void TraceMessageFilter::OnChildSupportsTracing() {
   has_child_ = true;
   TraceControllerImpl::GetInstance()->AddFilter(this);
 }
 
 void TraceMessageFilter::OnEndTracingAck(
     const std::vector<std::string>& known_categories) {
   // is_awaiting_end_ack_ should always be true here, but check in case the
   // child process is compromised.
+  DCHECK(is_awaiting_end_ack_);

[sky, 2012/06/06 20:16:32]

How about an else NOTREACHED instead. Or, remove the if entirely. Say comment on
110.

[jbates, 2012/06/06 21:22:17]

Done.

   if (is_awaiting_end_ack_) {
     is_awaiting_end_ack_ = false;
     TraceControllerImpl::GetInstance()->OnEndTracingAck(known_categories);
   }
 }
 
 void TraceMessageFilter::OnTraceDataCollected(const std::string& data) {
   scoped_refptr<base::RefCountedString> data_ptr(new base::RefCountedString());
   data_ptr->data() = data;
   TraceControllerImpl::GetInstance()->OnTraceDataCollected(data_ptr);
 }
 
 void TraceMessageFilter::OnTraceBufferFull() {
   TraceControllerImpl::GetInstance()->OnTraceBufferFull();
 }
 
 void TraceMessageFilter::OnTraceBufferPercentFullReply(float percent_full) {
-  if (is_awaiting_bpf_ack_) {
-    is_awaiting_bpf_ack_ = false;
+  DCHECK(is_awaiting_buffer_percent_full_ack_);
+  if (is_awaiting_buffer_percent_full_ack_) {
+    is_awaiting_buffer_percent_full_ack_ = false;
     TraceControllerImpl::GetInstance()->OnTraceBufferPercentFullReply(
         percent_full);
   }
 }