Last set of fixes to make the src/mojo/edk pass the page cycler.

mojo/edk/system/raw_channel.h

 // Copyright 2013 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.
 
 #ifndef MOJO_EDK_SYSTEM_RAW_CHANNEL_H_
 #define MOJO_EDK_SYSTEM_RAW_CHANNEL_H_
 
 #include <vector>
 
 #include "base/memory/scoped_ptr.h"
@@ skipping 51 matching lines @@
 
     // Called when a message is read. The delegate may not call back into this
     // object synchronously.
     virtual void OnReadMessage(
         const MessageInTransit::View& message_view,
         ScopedPlatformHandleVectorPtr platform_handles) = 0;
 
     // Called when there's a (fatal) error. The delegate may not call back into
     // this object synchronously.
     //
-    // For each raw channel, there'll be at most one |ERROR_READ_...| and at
-    // most one |ERROR_WRITE| notification. After |OnError(ERROR_READ_...)|,
-    // |OnReadMessage()| won't be called again.
+    // For each raw channel, there'll be at most one OnError notification.
     virtual void OnError(Error error) = 0;
 
    protected:
     virtual ~Delegate() {}
   };
 
   // Static factory method. |handle| should be a handle to a
   // (platform-appropriate) bidirectional communication channel (e.g., a socket
   // on POSIX, a named pipe on Windows).
   static RawChannel* Create(ScopedPlatformHandle handle);
@@ skipping 100 matching lines @@
     // TODO(vtl): Maybe this method should be const, but
     // |PlatformHandle::CloseIfNecessary()| isn't const (and actually modifies
     // state).
     void GetPlatformHandlesToSend(size_t* num_platform_handles,
                                   PlatformHandle** platform_handles,
                                   void** serialization_data);
 
     // Gets buffers to be written. These buffers will always come from the front
     // of |message_queue_|. Once they are completely written, the front
     // |MessageInTransit| should be popped (and destroyed); this is done in
-    // |OnWriteCompletedNoLock()|.
+    // |OnWriteCompletedInternalNoLock()|.
     void GetBuffers(std::vector<Buffer>* buffers);
 
     bool IsEmpty() const { return message_queue_.IsEmpty(); }
 
    private:
     friend class RawChannel;
 
     size_t serialized_platform_handle_size_;
 
     MessageInTransitQueue message_queue_;
@@ skipping 10 matching lines @@
   };
 
   RawChannel();
 
   // Shutdown must be called on the IO thread. This object deletes itself once
   // it's flushed all pending writes and insured that the other side of the pipe
   // read them.
   virtual ~RawChannel();
 
   // |result| must not be |IO_PENDING|. Must be called on the I/O thread WITHOUT
-  // |write_lock_| held. This object may be destroyed by this call.
+  // |write_lock_| held. This object may be destroyed by this call. This
+  // acquires |read_lock_| inside of it.
   void OnReadCompleted(IOResult io_result, size_t bytes_read);

[yzshen1, 2015/10/14 18:23:46]

Does it make sense tor remove On(Read|Write)Completed? (I understand they are
now still called on posix, but seems easy to change the callsites.)

[jam, 2015/10/14 20:01:44]

good point. done.

+  // Like above, except that the caller needs to acquire read_lock_ first.

[yzshen1, 2015/10/14 18:23:46]

style nit: please use |some_variable_name_| (and line 234).

[jam, 2015/10/14 20:01:44]

I stopped this since it's not in the style guide; just some preferences that
some people use. I find it obvious without :)

[yzshen1, 2015/10/14 21:37:00]

I see, although I think consistency within the file prevails.
Please consider this as an optional comment. :)

+  void OnReadCompletedNoLock(IOResult io_result, size_t bytes_read);
   // |result| must not be |IO_PENDING|. Must be called on the I/O thread WITHOUT
   // |write_lock_| held. This object may be destroyed by this call.
   void OnWriteCompleted(IOResult io_result,
                         size_t platform_handles_written,
                         size_t bytes_written);
+  // Like above, except that the caller needs to acquire write_lock_ first.
+  void OnWriteCompletedNoLock(IOResult io_result,
+                              size_t platform_handles_written,
+                              size_t bytes_written);
 
   // Serialize the read buffer into the given array so that it can be sent to
   // another process. Increments |num_valid_bytes_| by |additional_bytes_read|
   // before serialization..
   void SerializeReadBuffer(size_t additional_bytes_read,
                            std::vector<char>* buffer);
 
   // Serialize the pending messages to be written to the OS pipe to the given
   // buffer so that it can be sent to another process.
-  void SerializeWriteBuffer(std::vector<char>* buffer,
-                            size_t additional_bytes_written,
-                            size_t additional_platform_handles_written);
+  void SerializeWriteBuffer(size_t additional_bytes_written,
+                            size_t additional_platform_handles_written,
+                            std::vector<char>* buffer);
 
   base::MessageLoopForIO* message_loop_for_io() { return message_loop_for_io_; }
   base::Lock& write_lock() { return write_lock_; }
   base::Lock& read_lock() { return read_lock_; }
 
   // Should only be called on the I/O thread.
   ReadBuffer* read_buffer() { return read_buffer_.get(); }
 
   // Only called under |write_lock_|.
   WriteBuffer* write_buffer_no_lock() {
     write_lock_.AssertAcquired();
     return write_buffer_.get();
   }
 
+  bool pending_error() { return pending_error_; }
+
   // Adds |message| to the write message queue. Implementation subclasses may
   // override this to add any additional "control" messages needed. This is
   // called (on any thread) with |write_lock_| held.
   virtual void EnqueueMessageNoLock(scoped_ptr<MessageInTransit> message);
 
   // Handles any control messages targeted to the |RawChannel| (or
   // implementation subclass). Implementation subclasses may override this to
   // handle any implementation-specific control messages, but should call
   // |RawChannel::OnReadMessageForRawChannel()| for any remaining messages.
   // Returns true on success and false on error (e.g., invalid control message).
@@ skipping 85 matching lines @@
   // |write_lock_| held. This object may be destroyed by this call.
   void CallOnError(Delegate::Error error);
 
   void LockAndCallOnError(Delegate::Error error);
 
   // If |io_result| is |IO_SUCCESS|, updates the write buffer and schedules a
   // write operation to run later if there is more to write. If |io_result| is
   // failure or any other error occurs, cancels pending writes and returns
   // false. Must be called under |write_lock_| and only if |write_stopped_| is
   // false.
-  bool OnWriteCompletedNoLock(IOResult io_result,
-                              size_t platform_handles_written,
-                              size_t bytes_written);
+  bool OnWriteCompletedInternalNoLock(IOResult io_result,
+                                      size_t platform_handles_written,
+                                      size_t bytes_written);
 
   // Helper method to dispatch messages from the read buffer.
   // |did_dispatch_message| is true iff it dispatched any messages.
   // |stop_dispatching| is set to true if the code calling this should stop
   // dispatching, either because we hit an erorr or the delegate shutdown the
   // channel.
   void DispatchMessages(bool* did_dispatch_message, bool* stop_dispatching);
 
   void UpdateWriteBuffer(size_t platform_handles_written, size_t bytes_written);
 
@@ skipping 22 matching lines @@
 
   // If grabbing both locks, grab read first.
 
   base::Lock write_lock_;  // Protects the following members.
   bool write_ready_;
   bool write_stopped_;
   scoped_ptr<WriteBuffer> write_buffer_;
 
   bool error_occurred_;
 
+  // True iff a PostTask has been called to set an error. Can be written under
+  // either read or write lock. It's read with both acquired.
+  bool pending_error_;
+
   // This is used for posting tasks from write threads to the I/O thread. It
   // must only be accessed under |write_lock_|. The weak pointers it produces
   // are only used/invalidated on the I/O thread.
   base::WeakPtrFactory<RawChannel> weak_ptr_factory_;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(RawChannel);
 };
 
 }  // namespace edk
 }  // namespace mojo
 
 #endif  // MOJO_EDK_SYSTEM_RAW_CHANNEL_H_