Add support for runtime fall-back from o3d to o2d.

Add support for runtime fall-back from o3d to o2d iff o3d initialization fails for any reason.  This is still only supported on linux for GL renderer.  o2d mode can still be forced at compile time via the force_cairo=1 GYP_DEFINE.

Verified on Linux in o3d and o2d mode and on Mac.  Unit tests are still passing 100%.  Also confirmed o2d fall-back using NX on Linux.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=75776

[milligan, 2011-02-23 00:07:54]

Ready for code review.

[Tristan Schmelcher 2, 2011-02-23 02:08:51]

http://codereview.chromium.org/6538102/diff/4002/core/cross/client_info.h
File core/cross/client_info.h (right):

http://codereview.chromium.org/6538102/diff/4002/core/cross/client_info.h#new...
core/cross/client_info.h:140: void SetRenderer2d(bool render2d) {
This should be SetRender2d to match the property name.

http://codereview.chromium.org/6538102/diff/4002/core/cross/renderer.cc
File core/cross/renderer.cc (right):

http://codereview.chromium.org/6538102/diff/4002/core/cross/renderer.cc#newco...
core/cross/renderer.cc:832: // specific code.  This definition can be removed
when all platforms
I don't think you even need this now. It is not an error to declare a function
and never define it as long as there is no other code that references it. Since
the only call point is guarded by a SUPPORT_CAIRO, there shouldn't be any
problem with leaving it undefined.

http://codereview.chromium.org/6538102/diff/4002/plugin/cross/o3d_glue.cc
File plugin/cross/o3d_glue.cc (right):

http://codereview.chromium.org/6538102/diff/4002/plugin/cross/o3d_glue.cc#new...
plugin/cross/o3d_glue.cc:260: renderer_init_status_ =
o3d::Renderer::GPU_NOT_UP_TO_SPEC;
I preferred the previous code structure where this was only assigned once
o3d::CheckConfig() returned false. I think that is a bit clearer.

http://codereview.chromium.org/6538102/diff/4002/plugin/cross/o3d_glue.cc#new...
plugin/cross/o3d_glue.cc:278: Renderer::InitStatus temp_status =
renderer_->Init(display_window,
I don't think you need this temp_status variable.

[milligan, 2011-02-23 17:07:50]

Updated to incorporate feedback to date.  Please especially check the flow in
o2d_glue.cc to be sure you are happy with it.  Thanks.

[Tim H, 2011-02-23 18:22:16]

LGTM

[gangji, 2011-02-23 18:48:42]

LGTM.

[Tristan Schmelcher 2, 2011-02-23 19:27:30]

http://codereview.chromium.org/6538102/diff/13002/plugin/cross/o3d_glue.cc
File plugin/cross/o3d_glue.cc (right):

http://codereview.chromium.org/6538102/diff/13002/plugin/cross/o3d_glue.cc#ne...
plugin/cross/o3d_glue.cc:274: // To force cairo, pretend all previous renderer
initialization has failed.
You shouldn't need this, because renderer_init_status_ is set to UNINITIALIZED
in the constructor.

Hmm, though actually, it looks like CreateRenderer() could be called more than
once, so it may not be UNINITIALIZED at the start of this function. But the
original code does not appear to do the right thing in that case because it does
not delete the renderer_ before assigning the new one to it.

So actually, I think you should add this to the start of the function:

DeleteRenderer();
renderer_init_status_ = o3d::Renderer::UNINITIALIZED;

http://codereview.chromium.org/6538102/diff/13002/plugin/cross/o3d_glue.cc#ne...
plugin/cross/o3d_glue.cc:283: if (renderer_->Init(display_window, false) !=
o3d::Renderer::SUCCESS) {
I think we might as well just assign this result directly to
renderer_init_status_. If both 3D and 2D initialization fails, the error code
from the 2D failure is probably more appropriate since it is the more
fundamental error. For example, 3D may have failed with GPU_NOT_UP_TO_SPEC, but
it would not be accurate to say that that is the cause of initialization failure
because there is no requirement for any particular GPU when 2D is supported.

[Tristan Schmelcher 2, 2011-02-23 19:51:58]

LGTM