Added image pattern comparison logic for test interface and fixture.

remoting/test/test_video_renderer.h

 // Copyright 2015 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 REMOTING_TEST_TEST_VIDEO_RENDERER_H_
 #define REMOTING_TEST_TEST_VIDEO_RENDERER_H_
 
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "media/base/video_frame.h"
 #include "remoting/client/video_renderer.h"
 #include "remoting/protocol/session_config.h"
 #include "remoting/protocol/video_stub.h"
 
 namespace base {
 class Thread;
 class SingleThreadTaskRunner;
 }
 
 namespace webrtc {
 class DesktopFrame;
 class DesktopRect;
 }
 
-typedef uint32 RgbaColor;
-
 namespace remoting {
 namespace test {
 
+typedef uint32 RGBA32;

[Sergey Ulanov, 2015/07/09 19:43:15]

uint32_t . uint32 is a non-standard type used in older code. All new code should
use uint32_t.

[liaoyuke, 2015/07/09 23:21:01]

Done.

+
+// RGB Color triplets with alpha values as 0xff, and it is represented in ARGB
+// format.
+struct RGBColor {

[Sergey Ulanov, 2015/07/09 19:43:15]

this is a class, not struct.
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Structs_vs._...

[liaoyuke, 2015/07/09 23:21:01]

Done.

+ public:
+  RGBColor() : color_(0xFF000000) {}
+  explicit RGBColor(RGBA32 color) : color_(color) {}

[Sergey Ulanov, 2015/07/09 19:43:15]

Not sure why you need this. Default copy constructor should do the right thing.

[liaoyuke, 2015/07/09 23:21:02]

I have this default constructor only because I don't want to initialize it, but
you are right, I should delete this.

+  RGBColor(int red, int green, int blue);
+
+  // Return values on Red, Green, Blue channels.
+  int GetRed() const;

[Sergey Ulanov, 2015/07/09 19:43:15]

These can be inline, red() green() and blue()
See
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Names

[liaoyuke, 2015/07/09 23:21:01]

Done.

+  int GetGreen() const;
+  int GetBlue() const;
+
+  // Setters for color;
+  void SetRGB(int red, int green, int blue);

[Sergey Ulanov, 2015/07/09 19:43:15]

Call it Set()

[liaoyuke, 2015/07/09 23:21:01]

Done.

+  void SetRGB(RGBA32 color) { color_ = color; }
+
+ private:
+  RGBA32 color_;

[Sergey Ulanov, 2015/07/09 19:43:15]

would it be better to store red green and blue channels separately (e.g. using
uint8_t), that would simplify code.

[liaoyuke, 2015/07/09 23:21:01]

Yeah, I think it would simplify the code and be more efficient since GetRed,
GetGreen, GetBlue functions are frequently called.

+};
+
 // Processes video packets as they are received from the remote host. Must be
 // used from a thread running a message loop and this class will use that
 // message loop to execute the done callbacks passed by the caller of
 // ProcessVideoPacket.
 class TestVideoRenderer : public VideoRenderer, public protocol::VideoStub {
  public:
   TestVideoRenderer();
   ~TestVideoRenderer() override;
 
   // VideoRenderer interface.
@@ skipping 13 matching lines @@
 
   // Gets a weak pointer for this object.
   base::WeakPtr<TestVideoRenderer> GetWeakPtr() {
     return weak_factory_.GetWeakPtr();
   }
 
   // Set expected image pattern for comparison and the callback will be called
   // when the pattern is matched.
   void SetImagePatternAndMatchedCallback(
       const webrtc::DesktopRect& expected_rect,
-      const RgbaColor& expected_color,
+      const RGBA32& expected_color,
       const base::Closure& image_pattern_matched_callback);
 
  private:
   // The actual implementation resides in Core class.
   class Core;
   scoped_ptr<Core> core_;
 
   // Used to ensure TestVideoRenderer methods are called on the same thread.
   base::ThreadChecker thread_checker_;
 
   // Used to decode and process video packets.
   scoped_ptr<base::Thread>  video_decode_thread_;
 
   // Used to post tasks to video decode thread.
   scoped_refptr<base::SingleThreadTaskRunner> video_decode_task_runner_;
 
   // Used to weakly bind |this| to methods.
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<TestVideoRenderer> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(TestVideoRenderer);
 };
 
 }  // namespace test
 }  // namespace remoting
 
 #endif  // REMOTING_TEST_TEST_VIDEO_RENDERER_H_