Reland "Move a good set of gfx unit tests into gfx_unittests target."

Reland "Move a good set of gfx unit tests into gfx_unittests target."

It was reverted in r270600 because it broke compilation:
lib/libgfx.so:error: undefined reference to 'XCreateRegion'
lib/libgfx.so:error: undefined reference to 'XUnionRectWithRegion'
lib/libgfx.so:error: undefined reference to 'XPolygonRegion'

Original description:
This requires to major changes:

1- It requires the introduction of GfxTestSuite to allow us to
initialize the ResourceBundle which is required for some of these tests
to pass.
2- It requires to split gfx_unittests target out of gfx.gyp, into
gfx_tests.gyp. This is necessary because otherwise a circular dependency
between ui_base.gyp and gfx.gyp would arise.

The circular dependency is the form of:
gfx.gyp -> ui_base.gyp -> gfx.gyp

And is created because gfx_unittests now requires ResourceBundle to link
which is in ui_base target.

BUG=331829
TEST=ui_unittests,gfx_unittests
R=sadrul@chromium.org, danakj@chromium.org
TBR=ben@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272088

[tfarina, 2014-05-18 05:18:36]

Elliot, Sadrul can I have an extra pair of more experienced Linux eyes to look
at this and make sure I'm *not* going to break the build with this again?

Thanks in advance for reviewing this.

With this an -lX11 is added to the command line.

I verified it doing:

$ touch ui/gfx/test/run_all_unittests.cc
$ ninja -v -C out/Debug gfx_unittests

$ ldd out/Debug/gfx_unittests | grep X11
	libX11.so.6 => /usr/lib/x86_64-linux-gnu/libX11.so.6 (0x00007f429f7f1000)

$ ldd out/Debug/gfx_unittests | grep x11
	libx11_events_platform.so =>
/home/tfarina/chromium/src/out/Debug/lib/libx11_events_platform.so
(0x00007f09fd29a000)
	libgfx_x11.so => /home/tfarina/chromium/src/out/Debug/lib/libgfx_x11.so
(0x00007f09fb791000)

Is there any other tool/command I can run to making this going to land fine?

https://codereview.chromium.org/291753002/diff/1/ui/gfx/gfx_tests.gyp
File ui/gfx/gfx_tests.gyp (right):

https://codereview.chromium.org/291753002/diff/1/ui/gfx/gfx_tests.gyp#newcode81
ui/gfx/gfx_tests.gyp:81: '../../build/linux/system.gyp:x11',
I added this in the hope to fix the undefined X errors of libgfx.so (see CL
description), but I'm not sure this is the right fix or even if it is the fix. I
can't reproduce the error locally.

The full stack to the compilation error is in
https://codereview.chromium.org/290453002/

[tfarina, 2014-05-18 05:21:34]

On 2014/05/18 05:18:36, tfarina wrote:

> With this an -lX11 is added to the command line.
> 
> I verified it doing:
> 
> $ touch ui/gfx/test/run_all_unittests.cc
> $ ninja -v -C out/Debug gfx_unittests
> 
This should have been:
$ ninja -v -C out/Debug gfx_unittests | grep X11

> Is there any other tool/command I can run to making this going to land fine?
> 
Urr, s/to making/to make sure this is going ...

It's getting late :(

[sadrul, 2014-05-20 16:53:31]

https://codereview.chromium.org/291753002/diff/1/ui/gfx/gfx.gyp
File ui/gfx/gfx.gyp (right):

https://codereview.chromium.org/291753002/diff/1/ui/gfx/gfx.gyp#newcode375
ui/gfx/gfx.gyp:375: 'x/gfx_x11.gyp:gfx_x11',
path_x11.cc uses the XCreateRegion etc. functions. So perhaps we need to add
system.gyp:x11 dependency here, instead of in gfx_tests.gyp?

[tfarina, 2014-05-20 21:08:59]

https://codereview.chromium.org/291753002/diff/1/ui/gfx/gfx.gyp
File ui/gfx/gfx.gyp (right):

https://codereview.chromium.org/291753002/diff/1/ui/gfx/gfx.gyp#newcode375
ui/gfx/gfx.gyp:375: 'x/gfx_x11.gyp:gfx_x11',
On 2014/05/20 16:53:31, sadrul wrote:
> path_x11.cc uses the XCreateRegion etc. functions. So perhaps we need to add
> system.gyp:x11 dependency here, instead of in gfx_tests.gyp?

Done. But doing it here does not add -lX11 to command line. Strange :/

[sadrul, 2014-05-21 18:49:10]

https://codereview.chromium.org/291753002/diff/40001/ui/gfx/test/run_all_unit...
File ui/gfx/test/run_all_unittests.cc (right):

https://codereview.chromium.org/291753002/diff/40001/ui/gfx/test/run_all_unit...
ui/gfx/test/run_all_unittests.cc:31:
ui::ResourceBundle::InitSharedInstanceWithPakPath(pak_file);
Does the gfx tests actually use anything from the resource-bundle?

[tfarina, 2014-05-21 19:03:34]

https://codereview.chromium.org/291753002/diff/40001/ui/gfx/test/run_all_unit...
File ui/gfx/test/run_all_unittests.cc (right):

https://codereview.chromium.org/291753002/diff/40001/ui/gfx/test/run_all_unit...
ui/gfx/test/run_all_unittests.cc:31:
ui::ResourceBundle::InitSharedInstanceWithPakPath(pak_file);
On 2014/05/21 18:49:10, sadrul wrote:
> Does the gfx tests actually use anything from the resource-bundle?
Yes. Both font_unittests.cc and text_elider_unittests.cc end up calling things
that require ResourceBundle initialization.

[sadrul, 2014-05-21 19:10:21]

Do we need to update the BUILD.gn files?

The x11 dep-change lgtm

https://codereview.chromium.org/291753002/diff/40001/ui/gfx/test/run_all_unit...
File ui/gfx/test/run_all_unittests.cc (right):

https://codereview.chromium.org/291753002/diff/40001/ui/gfx/test/run_all_unit...
ui/gfx/test/run_all_unittests.cc:31:
ui::ResourceBundle::InitSharedInstanceWithPakPath(pak_file);
On 2014/05/21 19:03:35, tfarina wrote:
> On 2014/05/21 18:49:10, sadrul wrote:
> > Does the gfx tests actually use anything from the resource-bundle?
> Yes. Both font_unittests.cc and text_elider_unittests.cc end up calling things
> that require ResourceBundle initialization.

Ah, I see. That seems unfortunate. Perhaps we could clean these up.

[tfarina, 2014-05-21 19:28:53]

https://codereview.chromium.org/291753002/diff/40001/ui/gfx/test/run_all_unit...
File ui/gfx/test/run_all_unittests.cc (right):

https://codereview.chromium.org/291753002/diff/40001/ui/gfx/test/run_all_unit...
ui/gfx/test/run_all_unittests.cc:31:
ui::ResourceBundle::InitSharedInstanceWithPakPath(pak_file);
On 2014/05/21 19:10:21, sadrul wrote:
> On 2014/05/21 19:03:35, tfarina wrote:
> > On 2014/05/21 18:49:10, sadrul wrote:
> > > Does the gfx tests actually use anything from the resource-bundle?
> > Yes. Both font_unittests.cc and text_elider_unittests.cc end up calling
things
> > that require ResourceBundle initialization.
> 
> Ah, I see. That seems unfortunate. Perhaps we could clean these up.
For further reference the error (without this call) was:

[5967:5967:0521/162544:1047643224706:FATAL:font_list.cc(115)] Check failed:
!(g_default_font_description == NULL). SetDefaultFontDescription has not been
called.

And the list of tests that crashes without this call is:
27 tests crashed:
    FontTest.AvgWidths
    ImageSkiaTest.ArbitraryScaleFactor
    ImageSkiaTest.ArbitraryScaleFactorWithMissingResource
    ImageSkiaTest.BackedBySameObjectAs
    ImageSkiaTest.DynamicSource
    ImageSkiaTest.EmptyOnThreadTest
    ImageSkiaTest.FixedSource
    ImageSkiaTest.GetBitmap
    ImageSkiaTest.GetBitmapFromEmpty
    ImageSkiaTest.ManyRepsPerScaleFactor
    ImageSkiaTest.SourceOnThreadTest
    ImageSkiaTest.StaticOnThreadTest
    ImageSkiaTest.Unscaled
    ImageSkiaTest.UnscaledImageForArbitraryScaleFactor
    TextEliderTest.ElideEmail
    TextEliderTest.ElideEmailMoreSpace
    TextEliderTest.ElideRectangleText
    TextEliderTest.ElideRectangleTextCheckLineWidth
    TextEliderTest.ElideRectangleTextLongWords
    TextEliderTest.ElideRectangleTextPunctuation
    TextEliderTest.ElideTextEllipsis
    TextEliderTest.ElideTextEllipsisFront
    TextEliderTest.ElideTextLongStrings
    TextEliderTest.ElideTextSurrogatePairs
    TextEliderTest.ElideTextTruncate
    TextEliderTest.TestFilenameEliding
    TextUtilsTest.GetStringWidth

Hence this call to InitSharedInstanceWithPakPath() be extremely important.

[tfarina, 2014-05-21 19:38:14]

I will do GN separately, as this change is already too risky.

And I don't want to deal with two different build systems in the same change.

I tested gn build locally and it works fine as is.

[tfarina, 2014-05-22 04:11:30]

Committed patchset #3 manually as r272088 (presubmit successful).