Submitting ByteStream chrome class for Google C++ Readability Review.

content/browser/download/byte_stream.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/download/byte_stream.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/memory/weak_ptr.h"
 #include "base/memory/ref_counted.h"
@@ skipping 15 matching lines @@
 class ByteStreamReaderImpl;
 
 // A poor man's weak pointer; a RefCountedThreadSafe boolean that can be
 // cleared in an object destructor and accessed to check for object
 // existence.  We can't use weak pointers because they're tightly tied to
 // threads rather than task runners.
 // TODO(rdsmith): A better solution would be extending weak pointers
 // to support SequencedTaskRunners.
 struct LifetimeFlag : public base::RefCountedThreadSafe<LifetimeFlag> {
  public:
   LifetimeFlag() : is_alive_(true) { }

[erikmc, 2012/06/18 19:25:44]

Structs have different naming rules for data members.  Please remove the
trailing underscore in is_alive_.

http://www.corp.google.com/eng/doc/cppguide.xml?showone=Variable_Names#Variab...

[Randy Smith (Not in Mondays), 2012/06/18 20:38:24]

Done.

   bool is_alive_;
 
  protected:
   friend class base::RefCountedThreadSafe<LifetimeFlag>;
   virtual ~LifetimeFlag() { }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(LifetimeFlag);
 };
 
 // For both ByteStreamWriterImpl and ByteStreamReaderImpl, Construction and
 // SetPeer may happen anywhere; all other operations on each class must
 // happen in the context of their SequencedTaskRunner.
 class ByteStreamWriterImpl : public content::ByteStreamWriter {
  public:
   ByteStreamWriterImpl(scoped_refptr<base::SequencedTaskRunner> task_runner,

[erikmc, 2012/06/18 19:25:44]

align inputs here

[Randy Smith (Not in Mondays), 2012/06/18 20:38:24]

Huh.  Wonder how I managed that.  Done.

                       scoped_refptr<LifetimeFlag> lifetime_flag,
                       size_t buffer_size);
   virtual ~ByteStreamWriterImpl();
 
   // Must be called before any operations are performed.
   void SetPeer(ByteStreamReaderImpl* peer,
                scoped_refptr<base::SequencedTaskRunner> peer_task_runner,
                scoped_refptr<LifetimeFlag> peer_lifetime_flag);
 
   // Overridden from ByteStreamWriter.
@@ skipping 368 matching lines @@
   ByteStreamReaderImpl* out = new ByteStreamReaderImpl(
       output_task_runner, output_flag, buffer_size);
 
   in->SetPeer(out, output_task_runner, output_flag);
   out->SetPeer(in, input_task_runner, input_flag);
   input->reset(in);
   output->reset(out);
 }
 
 }  // namespace content
+

[erikmc, 2012/06/18 19:25:44]

Please remove empty line 441.

[Randy Smith (Not in Mondays), 2012/06/18 20:38:24]

Yep--this was to get Rietveld to "take" the patch.  Done, and done in
byte_stream.h as well; I was "lucky" enough to have this error to correct in
byte_stream_unittest.cc when I uploaded :-}.