[Chromoting] Unit Tests for GlRenderer

[Chromoting] Unit Tests for GlRenderer

This CL separates the OpenGL drawing logic from GlRenderer and adds unit tests
for delegate callback and redraw scheduling logic.

BUG=385924
Committed: https://crrev.com/e4c0f786357df0eea6b5688b40d6a088aef28580
Cr-Commit-Position: refs/heads/master@{#411152}

[Yuwei, 2016-07-29 00:05:53]

Patchset #1 (id:1) has been deleted

[Yuwei, 2016-07-29 00:06:10]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 00:06:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yuwei, 2016-07-29 00:07:01]

yuweih@chromium.org changed reviewers:
+ sergeyu@chromium.org

[Yuwei, 2016-07-29 00:07:02]

ptal

https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_desk...
File remoting/client/gl_desktop.h (right):

https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_desk...
remoting/client/gl_desktop.h:29: void SetVideoFrame(const webrtc::DesktopFrame&
frame);
Changed this to const ref since GlDesktop no longer needs to store the previous
desktop frame...

[commit-bot: I haz the power, 2016-07-29 01:10:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 01:10:47]

Dry run: This issue passed the CQ dry run.

[Sergey Ulanov, 2016-08-01 21:33:05]

https://codereview.webrtc.org/2196493002/diff/20001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.webrtc.org/2196493002/diff/20001/remoting/client/BUILD.gn#...
remoting/client/BUILD.gn:90: "gl_renderer.cc",
This would meant that gl_renderer.cc is compiled twice, which we need to avoid.
Instead you should add dependency on opengl_renderer. This list shouldn't
include any files beside *unittest.cc 
Also I think you want to add gl_renderer_unittest.cc only on platforms that it
supports. Potentially it should be possible to make it work on linux, and mac as
well - that would make it easier to debug these tests.

https://codereview.webrtc.org/2196493002/diff/20001/remoting/client/gl_render...
File remoting/client/gl_renderer_core.h (right):

https://codereview.webrtc.org/2196493002/diff/20001/remoting/client/gl_render...
remoting/client/gl_renderer_core.h:22: class GlRendererCore {
With this change GlRenderer becomes just a think wrapper for GlRendererCore and
then you supply fake GlRendererCore implementation for the test. This means that
the tests only run the code in GlRenderer and so they are not very useful. I
think it would be better to run the actual renderer and execute code from the
other Gl* classes. Then the test could render everything to a framebuffer, read
it back and verify that everything is rendered properly.

https://codereview.webrtc.org/2196493002/diff/20001/remoting/client/gl_render...
remoting/client/gl_renderer_core.h:24: static std::unique_ptr<GlRendererCore>
CreateCore();
We use static interface constructors like this for platform-specific code, but
not to mock interfaces for testing. If you need to mock a layer under the class
being tested then the class should be getting a pointer to the lower layer
implementation somehow explicitly, instead of calling a static constructor.
But see my previous comment, I don't think we actually need to mock anything
under GlRenderer.

[Yuwei, 2016-08-01 22:04:40]

https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_rend...
File remoting/client/gl_renderer_core.h (right):

https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_rend...
remoting/client/gl_renderer_core.h:22: class GlRendererCore {
On 2016/08/01 21:33:05, Sergey Ulanov wrote:
> With this change GlRenderer becomes just a think wrapper for GlRendererCore
and
> then you supply fake GlRendererCore implementation for the test. This means
that
> the tests only run the code in GlRenderer and so they are not very useful. I
> think it would be better to run the actual renderer and execute code from the
> other Gl* classes. Then the test could render everything to a framebuffer,
read
> it back and verify that everything is rendered properly.

My initial thought was to test the delegate calls and render scheduling. Testing
the actual rendering output sounds like an integration test and would be more
complex.

I can still manage to use Mesa OpenGL or something to get the code compile in
different platforms. I can spend some time making tests for the render result if
you think that's useful.

[Yuwei, 2016-08-01 22:57:43]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 22:58:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sergey Ulanov, 2016-08-01 23:46:01]

On 2016/08/01 22:04:40, Yuwei wrote:
>
https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_rend...
> File remoting/client/gl_renderer_core.h (right):
> 
>
https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_rend...
> remoting/client/gl_renderer_core.h:22: class GlRendererCore {
> On 2016/08/01 21:33:05, Sergey Ulanov wrote:
> > With this change GlRenderer becomes just a think wrapper for GlRendererCore
> and
> > then you supply fake GlRendererCore implementation for the test. This means
> that
> > the tests only run the code in GlRenderer and so they are not very useful. I
> > think it would be better to run the actual renderer and execute code from
the
> > other Gl* classes. Then the test could render everything to a framebuffer,
> read
> > it back and verify that everything is rendered properly.
> 
> My initial thought was to test the delegate calls and render scheduling.
Testing
> the actual rendering output sounds like an integration test and would be more
> complex.

Whether that is a integration test or unittest depends on how you define a
"unit" :-). There is no rule that says that each class is a separate "unit". In
practice it rarely makes sense to test a single class in isolation mocking all
underlying layers. Most of our unittests execute code in more than one class and
verify that they work together correctly. Some random examples:
 1. SSL sockets unittests (see net/socket/ssl_client_socket_unittest.cc) use
real TCP sockets (they are connected to a separate test server process).
 2. Codec unittests in remoting/codec run real libvpx encoder/decoder.

In this case I think it's best to look at GlRenderer together with all Gl*
classes as a single unit and test them together. And the approach I suggested
should allow to test them without adding any artificial glue code like you do in
this CL. Also verifying the output of the test is not super important (i.e. that
everything is rendered correctly) - it can be added later. Even without that the
test runs the whole renderer together would be useful, it would ensure that the
renderer doesn't crash and you can still verify some conditions, e.g. that the
frames are scheduled correctly.

> 
> I can still manage to use Mesa OpenGL or something to get the code compile in
> different platforms. I can spend some time making tests for the render result
if
> you think that's useful.

I believe there should be existing tests in chromium that use OpenGL (e.g. in
ui/gl/ and in cc/). It looks like they use system OpenGL on linux and mesa on
windows, see
https://codesearch.chromium.org/chromium/src/ui/gl/BUILD.gn?rcl=0&l=239 . It
should be possible to apply the same approach in our unittests. I think it would
be useful (e.g. it would make it easier for you to develop the tests), but don't
waste much time on it if it turns out to be non-trivial change.

[Yuwei, 2016-08-01 23:54:33]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 23:54:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 00:12:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 00:12:56]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Yuwei, 2016-08-02 00:13:10]

PTAL

On 2016/08/01 23:46:01, Sergey Ulanov wrote:
> On 2016/08/01 22:04:40, Yuwei wrote:
> >
>
https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_rend...
> > File remoting/client/gl_renderer_core.h (right):
> > 
> >
>
https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_rend...
> > remoting/client/gl_renderer_core.h:22: class GlRendererCore {
> > On 2016/08/01 21:33:05, Sergey Ulanov wrote:
> > > With this change GlRenderer becomes just a think wrapper for
GlRendererCore
> > and
> > > then you supply fake GlRendererCore implementation for the test. This
means
> > that
> > > the tests only run the code in GlRenderer and so they are not very useful.
I
> > > think it would be better to run the actual renderer and execute code from
> the
> > > other Gl* classes. Then the test could render everything to a framebuffer,
> > read
> > > it back and verify that everything is rendered properly.
> > 
> > My initial thought was to test the delegate calls and render scheduling.
> Testing
> > the actual rendering output sounds like an integration test and would be
more
> > complex.
> 
> Whether that is a integration test or unittest depends on how you define a
> "unit" :-). There is no rule that says that each class is a separate "unit".
In
> practice it rarely makes sense to test a single class in isolation mocking all
> underlying layers. Most of our unittests execute code in more than one class
and
> verify that they work together correctly. Some random examples:
>  1. SSL sockets unittests (see net/socket/ssl_client_socket_unittest.cc) use
> real TCP sockets (they are connected to a separate test server process).
>  2. Codec unittests in remoting/codec run real libvpx encoder/decoder.

Acknowledged.

> In this case I think it's best to look at GlRenderer together with all Gl*
> classes as a single unit and test them together. And the approach I suggested
> should allow to test them without adding any artificial glue code like you do
in
> this CL. Also verifying the output of the test is not super important (i.e.
that
> everything is rendered correctly) - it can be added later. Even without that
the
> test runs the whole renderer together would be useful, it would ensure that
the
> renderer doesn't crash and you can still verify some conditions, e.g. that the
> frames are scheduled correctly.

SGTM. Looks like using system OpenGL doesn't crash. I think there should be
OpenGL errors for not having an OpenGL context but they just do nothing other
than setting the error flag.

So maybe for now I just write tests for delegate calls and render scheduling and
leave a TODO for adding tests for rendering in the future?

> > 
> > I can still manage to use Mesa OpenGL or something to get the code compile
in
> > different platforms. I can spend some time making tests for the render
result
> if
> > you think that's useful.
> 
> I believe there should be existing tests in chromium that use OpenGL (e.g. in
> ui/gl/ and in cc/). It looks like they use system OpenGL on linux and mesa on
> windows, see
> https://codesearch.chromium.org/chromium/src/ui/gl/BUILD.gn?rcl=0&l=239 . It
> should be possible to apply the same approach in our unittests. I think it
would
> be useful (e.g. it would make it easier for you to develop the tests), but
don't
> waste much time on it if it turns out to be non-trivial change.

The Windows OpenGL lib doesn't quite work the same way as those in POSIX
system... I'll take a look to see if there is any change I need to make for
Windows. We will need to establish OpenGL context if we want to test rendering,
which will be platform specific.

https://codereview.chromium.org/2196493002/diff/20001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2196493002/diff/20001/remoting/client/BUILD.g...
remoting/client/BUILD.gn:90: "gl_renderer.cc",
On 2016/08/01 21:33:05, Sergey Ulanov wrote:
> This would meant that gl_renderer.cc is compiled twice, which we need to
avoid.
> Instead you should add dependency on opengl_renderer. This list shouldn't
> include any files beside *unittest.cc 
> Also I think you want to add gl_renderer_unittest.cc only on platforms that it
> supports. Potentially it should be possible to make it work on linux, and mac
as
> well - that would make it easier to debug these tests.

Obsolete.

https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_rend...
File remoting/client/gl_renderer_core.h (right):

https://codereview.chromium.org/2196493002/diff/20001/remoting/client/gl_rend...
remoting/client/gl_renderer_core.h:24: static std::unique_ptr<GlRendererCore>
CreateCore();
On 2016/08/01 21:33:05, Sergey Ulanov wrote:
> We use static interface constructors like this for platform-specific code, but
> not to mock interfaces for testing. If you need to mock a layer under the
class
> being tested then the class should be getting a pointer to the lower layer
> implementation somehow explicitly, instead of calling a static constructor.
> But see my previous comment, I don't think we actually need to mock anything
> under GlRenderer.

Obsolete.

[Sergey Ulanov, 2016-08-02 00:28:15]

If you just make the unittests run on linux that's already great. I wouldn't
worry much about windows.

[Yuwei, 2016-08-02 00:28:53]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 00:29:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yuwei, 2016-08-02 00:52:30]

On 2016/08/02 00:28:15, Sergey Ulanov wrote:
> If you just make the unittests run on linux that's already great. I wouldn't
> worry much about windows.

Acknowledged.

Looks like there is something called libANGLE that can translate OpenGL ES API's
to the native API's. Should we just use this in the test? Just not sure whether
it will break iOS builder since there is no usage of OS_IOS...

[commit-bot: I haz the power, 2016-08-02 01:07:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 01:07:17]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Yuwei, 2016-08-02 01:16:43]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 01:17:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 02:08:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 02:08:38]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Yuwei, 2016-08-02 03:25:16]

On 2016/08/02 00:52:30, Yuwei wrote:
> On 2016/08/02 00:28:15, Sergey Ulanov wrote:
> > If you just make the unittests run on linux that's already great. I wouldn't
> > worry much about windows.
> 
> Acknowledged.
> 
> Looks like there is something called libANGLE that can translate OpenGL ES
API's
> to the native API's. Should we just use this in the test? Just not sure
whether
> it will break iOS builder since there is no usage of OS_IOS...

NVM... I still get linking error when trying libANGLE... I will just use the
system OpenGL in Linux.

[Yuwei, 2016-08-02 18:03:33]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 18:04:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yuwei, 2016-08-02 18:05:28]

Patchset #7 (id:140001) has been deleted

[commit-bot: I haz the power, 2016-08-02 18:08:12]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Yuwei, 2016-08-02 18:09:41]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 18:10:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yuwei, 2016-08-02 18:10:54]

On 2016/08/02 00:28:15, Sergey Ulanov wrote:
> If you just make the unittests run on linux that's already great. I wouldn't
> worry much about windows.

Done.

[Yuwei, 2016-08-02 19:02:06]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 19:02:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 20:42:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 20:42:48]

Dry run: This issue passed the CQ dry run.

[Yuwei, 2016-08-03 00:36:48]

https://codereview.chromium.org/2196493002/diff/200001/remoting/client/gl_des...
File remoting/client/gl_desktop.h (right):

https://codereview.chromium.org/2196493002/diff/200001/remoting/client/gl_des...
remoting/client/gl_desktop.h:29: void SetVideoFrame(const webrtc::DesktopFrame&
frame);
I'll hold this change for the multi-texture desktop CL.

[Yuwei, 2016-08-03 00:37:14]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-03 00:38:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 01:35:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-03 01:35:02]

Dry run: This issue passed the CQ dry run.

[Yuwei, 2016-08-09 00:53:53]

Any update for this CL?

[Sergey Ulanov, 2016-08-10 15:35:06]

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/BUILD....
remoting/client/BUILD.gn:115: if (is_linux) {
Can we run this test on android? If not then it should be explained in the
comments. Also it's better to always compile the test, but skip it if it doesn't
work on the current platform. I.e. replace this condition in the build file with
an #ifdef in the test source.

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_cur...
File remoting/client/gl_cursor.h (right):

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_cur...
remoting/client/gl_cursor.h:8: #include <stdint.h>
cstdint

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
File remoting/client/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
remoting/client/gl_renderer_unittest.cc:55: int canvas_height_ = 0;
make these private and add methods to access them

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
remoting/client/gl_renderer_unittest.cc:56: private:
empty line before private:

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
remoting/client/gl_renderer_unittest.cc:76: base::MessageLoop message_loop_;
move this above renderer_ so it's destroyed last.

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
remoting/client/gl_renderer_unittest.cc:191: // TODO(yuweih): Add tests for the
rendering process.
Not clear what you mean by "rendering process". Did you mean that we need tests
that would validate rendered screen content?

[Sergey Ulanov, 2016-08-10 15:35:22]

lgtm when my comments are addressed

[Yuwei, 2016-08-10 18:01:18]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 18:02:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yuwei, 2016-08-10 18:15:28]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 18:16:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yuwei, 2016-08-10 19:17:26]

The CQ bit was checked by yuweih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 19:18:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 21:24:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-10 21:24:03]

Dry run: This issue passed the CQ dry run.

[Yuwei, 2016-08-10 21:31:01]

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/BUILD.gn
File remoting/client/BUILD.gn (right):

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/BUILD....
remoting/client/BUILD.gn:115: if (is_linux) {
On 2016/08/10 15:35:06, Sergey Ulanov wrote:
> Can we run this test on android? If not then it should be explained in the
> comments. Also it's better to always compile the test, but skip it if it
doesn't
> work on the current platform. I.e. replace this condition in the build file
with
> an #ifdef in the test source.

Managed to get it compiled on all platforms. It is safe as long as it compiles
and no OpenGL function calls happen during the test.

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_cur...
File remoting/client/gl_cursor.h (right):

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_cur...
remoting/client/gl_cursor.h:8: #include <stdint.h>
On 2016/08/10 15:35:06, Sergey Ulanov wrote:
> cstdint

Done.

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
File remoting/client/gl_renderer_unittest.cc (right):

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
remoting/client/gl_renderer_unittest.cc:55: int canvas_height_ = 0;
On 2016/08/10 15:35:06, Sergey Ulanov wrote:
> make these private and add methods to access them

Done.

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
remoting/client/gl_renderer_unittest.cc:56: private:
On 2016/08/10 15:35:06, Sergey Ulanov wrote:
> empty line before private:

Done.

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
remoting/client/gl_renderer_unittest.cc:76: base::MessageLoop message_loop_;
On 2016/08/10 15:35:06, Sergey Ulanov wrote:
> move this above renderer_ so it's destroyed last.

Done.

https://codereview.chromium.org/2196493002/diff/260001/remoting/client/gl_ren...
remoting/client/gl_renderer_unittest.cc:191: // TODO(yuweih): Add tests for the
rendering process.
On 2016/08/10 15:35:06, Sergey Ulanov wrote:
> Not clear what you mean by "rendering process". Did you mean that we need
tests
> that would validate rendered screen content?

Yes. Changed the comment.

[Yuwei, 2016-08-10 21:33:09]

The CQ bit was checked by yuweih@chromium.org

[Yuwei, 2016-08-10 21:33:10]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/2196493002/#ps320001
(title: "Don't draw when canvas is not set")

[commit-bot: I haz the power, 2016-08-10 21:34:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 21:41:22]

Committed patchset #15 (id:320001)

[commit-bot: I haz the power, 2016-08-10 21:42:55]

Description was changed from

==========
[Chromoting] Unit Tests for GlRenderer

This CL separates the OpenGL drawing logic from GlRenderer and adds unit tests
for delegate callback and redraw scheduling logic.

BUG=385924
==========

to

==========
[Chromoting] Unit Tests for GlRenderer

This CL separates the OpenGL drawing logic from GlRenderer and adds unit tests
for delegate callback and redraw scheduling logic.

BUG=385924

Committed: https://crrev.com/e4c0f786357df0eea6b5688b40d6a088aef28580
Cr-Commit-Position: refs/heads/master@{#411152}
==========

[commit-bot: I haz the power, 2016-08-10 21:42:56]

Patchset 15 (id:??) landed as
https://crrev.com/e4c0f786357df0eea6b5688b40d6a088aef28580
Cr-Commit-Position: refs/heads/master@{#411152}