Don't load resources for test

The test framework doesn't really need to load resources and translations. This removes loading and changes the odd test that wasn't working anymore due to this change.

BUG=

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

[Tiger, 2013-07-11 15:16:59]

This review is to fix the outstanding issue in
<https://codereview.chromium.org/16520007/> and is thus related to bug
<http://code.google.com/p/chromium/issues/detail?id=248867>.

[abarth-chromium, 2013-07-11 17:24:01]

I'm not the right person to review this CL.

[Nico, 2013-07-11 22:18:19]

jochen knows the bundle / data pack situation better than I do. This seems to
change things so that some non-bundled executable reads the pak from within
content shell, which doesn't look right though (if content shell wasn't built
yet but the other test binary was, the pak won't exist in the bundle -- you want
to pick it up from the build directory in nonbundled executables)

[Tiger, 2013-07-12 09:48:24]

On 2013/07/11 22:18:19, Nico wrote:
> jochen knows the bundle / data pack situation better than I do. This seems to
> change things so that some non-bundled executable reads the pak from within
> content shell, which doesn't look right though (if content shell wasn't built
> yet but the other test binary was, the pak won't exist in the bundle -- you
want
> to pick it up from the build directory in nonbundled executables)

Yeah this changes so that non-bundled test (or at least things using the
TestWebKitPlatformSupport) can read in data resources (broken image and textarea
resizer). I'm not sure why these two resources are so special. From what I can
see this code isn't in use by neither content shell nor chromium. The
TestWebKitPlatformSupport::GetLocalizedString function already works like this
and is in use by some webkit unit tests already. There is however some code to
skip getting strings (if run from unit tests):

  // |g_resource_data_pack| is null on unit tests.
  // But som unit tests reach GetLocalizedString().
  if (!g_resource_data_pack)
    return base::string16();

However, there is a LOG(FATAL) if we fail to read this data pack, so this code
probably isn't in use. But perhaps this is a better way to go also for data
resources? But I can't see are there any test applications, that uses this code,
that are built as a bundle for mac?

Hopefully someone with more knowledge on this can give clarify things.

[jochen (gone - plz use gerrit), 2013-07-12 19:38:46]

I think it would be best if we could avoid reading resources at all during unit
tests.

We needed the resource file for running layout tests in DRT, but since that's
gone, it should be possible to just return some fake result if something wants
to read the images.

[Tiger, 2013-07-12 23:25:00]

On 2013/07/12 19:38:46, jochen wrote:
> I think it would be best if we could avoid reading resources at all during
unit
> tests.
> 
> We needed the resource file for running layout tests in DRT, but since that's
> gone, it should be possible to just return some fake result if something wants
> to read the images.

So both TestWebKitPlatformSupport::GetLocalizedString and
TestWebKitPlatformSupport::GetDataResource could return fake results for all
platforms at all times? That could simplify things a bit. I'm still not entirely
sure in what situations these functions are used. Do these functions need to
return a correct string or correct resource in some settings?

[jochen (gone - plz use gerrit), 2013-07-15 07:06:35]

On 2013/07/12 23:25:00, Tiger wrote:
> On 2013/07/12 19:38:46, jochen wrote:
> > I think it would be best if we could avoid reading resources at all during
> unit
> > tests.
> > 
> > We needed the resource file for running layout tests in DRT, but since
that's
> > gone, it should be possible to just return some fake result if something
wants
> > to read the images.
> 
> So both TestWebKitPlatformSupport::GetLocalizedString and
> TestWebKitPlatformSupport::GetDataResource could return fake results for all
> platforms at all times? That could simplify things a bit. I'm still not
entirely
> sure in what situations these functions are used. Do these functions need to
> return a correct string or correct resource in some settings?

I would just try returning an empty string / invalid resource and see if
something breaks. If that's indeed the case, we should consider changing the
test that triggers this.

[Tiger, 2013-07-15 14:14:12]

On 2013/07/15 07:06:35, jochen wrote:
> On 2013/07/12 23:25:00, Tiger wrote:
> > On 2013/07/12 19:38:46, jochen wrote:
> > > I think it would be best if we could avoid reading resources at all during
> > unit
> > > tests.
> > > 
> > > We needed the resource file for running layout tests in DRT, but since
> that's
> > > gone, it should be possible to just return some fake result if something
> wants
> > > to read the images.
> > 
> > So both TestWebKitPlatformSupport::GetLocalizedString and
> > TestWebKitPlatformSupport::GetDataResource could return fake results for all
> > platforms at all times? That could simplify things a bit. I'm still not
> entirely
> > sure in what situations these functions are used. Do these functions need to
> > return a correct string or correct resource in some settings?
> 
> I would just try returning an empty string / invalid resource and see if
> something breaks. If that's indeed the case, we should consider changing the
> test that triggers this.

On Mac, as far as I can see, there seem to be only one test that starts failing
with this solution -- the LocaleMacTest.formatWeek test seem to depend on having
translations available. I'll look into linux as well.

[jochen (gone - plz use gerrit), 2013-07-15 14:28:14]

+keishi who wrote the test

what exactly is failing?

[Tiger, 2013-07-15 14:34:50]

On 2013/07/15 14:28:14, jochen wrote:
> +keishi who wrote the test
> 
> what exactly is failing?

It wants to read a localized string (through):

0  0x04b4f91f in TestWebKitPlatformSupport::GetLocalizedString (this=0xe020000,
message_id=18094) at ../../webkit/support/platform_support_mac.mm:55
#1  0x04b4f9a7 in non-virtual thunk to
TestWebKitPlatformSupport::GetLocalizedString(int) (this=0xe020004,
message_id=18094) at ../../webkit/support/platform_support_mac.mm:57
#2  0x020bfd60 in webkit_glue::WebKitPlatformSupportImpl::queryLocalizedString
(this=0xe020004, name=WebKit::WebLocalizedString::WeekFormatTemplate) at
../../webkit/glue/webkitplatformsupport_impl.cc:690
#3  0x04b51872 in TestWebKitPlatformSupport::queryLocalizedString
(this=0xe020000, name=WebKit::WebLocalizedString::WeekFormatTemplate) at
../../webkit/support/test_webkit_platform_support.cc:245
#4  0x04b51927 in non-virtual thunk to
TestWebKitPlatformSupport::queryLocalizedString(WebKit::WebLocalizedString::Name)
(this=0xe020004, name=WebKit::WebLocalizedString::WeekFormatTemplate) at
../../webkit/support/test_webkit_platform_support.cc:247
#5  0x0278bfda in WebCore::query
(name=WebKit::WebLocalizedString::WeekFormatTemplate) at
../../third_party/WebKit/Source/core/platform/chromium/LocalizedStringsChromium.cpp:51
#6  0x0278ce1e in WebCore::weekFormatInLDML () at
../../third_party/WebKit/Source/core/platform/chromium/LocalizedStringsChromium.cpp:257
#7  0x02a9fb6e in WebCore::Locale::formatDateTime (this=0xcce04e0,
date=@0xbffff628, formatType=WebCore::Locale::FormatTypeUnspecified) at
../../third_party/WebKit/Source/core/platform/text/PlatformLocale.cpp:350
#8  0x003daa4f in LocaleMacTest::formatWeek (this=0xcc73f70,
localeString=@0xbffff710, isoString=@0xbffff708) at
../../third_party/WebKit/Source/WebKit/chromium/tests/LocaleMacTest.cpp:78

and compare it with the expected strings: "Week 04, 2005", "Week 52, 2005". But
now TestWebKitPlatformSupport::GetLocalizedString returns an empty string
instead. The other LocaleMacTests works just fine though.

[jochen (gone - plz use gerrit), 2013-07-15 15:03:15]

On 2013/07/15 14:34:50, Tiger wrote:
> On 2013/07/15 14:28:14, jochen wrote:
> > +keishi who wrote the test
> > 
> > what exactly is failing?
> 
> It wants to read a localized string (through):
> 
> 0  0x04b4f91f in TestWebKitPlatformSupport::GetLocalizedString
(this=0xe020000,
> message_id=18094) at ../../webkit/support/platform_support_mac.mm:55
> #1  0x04b4f9a7 in non-virtual thunk to
> TestWebKitPlatformSupport::GetLocalizedString(int) (this=0xe020004,
> message_id=18094) at ../../webkit/support/platform_support_mac.mm:57
> #2  0x020bfd60 in webkit_glue::WebKitPlatformSupportImpl::queryLocalizedString
> (this=0xe020004, name=WebKit::WebLocalizedString::WeekFormatTemplate) at
> ../../webkit/glue/webkitplatformsupport_impl.cc:690
> #3  0x04b51872 in TestWebKitPlatformSupport::queryLocalizedString
> (this=0xe020000, name=WebKit::WebLocalizedString::WeekFormatTemplate) at
> ../../webkit/support/test_webkit_platform_support.cc:245
> #4  0x04b51927 in non-virtual thunk to
>
TestWebKitPlatformSupport::queryLocalizedString(WebKit::WebLocalizedString::Name)
> (this=0xe020004, name=WebKit::WebLocalizedString::WeekFormatTemplate) at
> ../../webkit/support/test_webkit_platform_support.cc:247
> #5  0x0278bfda in WebCore::query
> (name=WebKit::WebLocalizedString::WeekFormatTemplate) at
>
../../third_party/WebKit/Source/core/platform/chromium/LocalizedStringsChromium.cpp:51
> #6  0x0278ce1e in WebCore::weekFormatInLDML () at
>
../../third_party/WebKit/Source/core/platform/chromium/LocalizedStringsChromium.cpp:257
> #7  0x02a9fb6e in WebCore::Locale::formatDateTime (this=0xcce04e0,
> date=@0xbffff628, formatType=WebCore::Locale::FormatTypeUnspecified) at
> ../../third_party/WebKit/Source/core/platform/text/PlatformLocale.cpp:350
> #8  0x003daa4f in LocaleMacTest::formatWeek (this=0xcc73f70,
> localeString=@0xbffff710, isoString=@0xbffff708) at
> ../../third_party/WebKit/Source/WebKit/chromium/tests/LocaleMacTest.cpp:78
> 
> and compare it with the expected strings: "Week 04, 2005", "Week 52, 2005".
But
> now TestWebKitPlatformSupport::GetLocalizedString returns an empty string
> instead. The other LocaleMacTests works just fine though.

That test would fail if it got a localized value, so I guess hardcoding the
template would be an option.

[Tiger, 2013-07-15 16:13:34]

> I would just try returning an empty string / invalid resource and see if
> something breaks. If that's indeed the case, we should consider changing the
> test that triggers this.

This seems to be fine on linux.

[keishi, 2013-07-16 02:28:05]

On 2013/07/15 16:13:34, Tiger wrote:
> > I would just try returning an empty string / invalid resource and see if
> > something breaks. If that's indeed the case, we should consider changing the
> > test that triggers this.
> 
> This seems to be fine on linux.

We have locale dependent behavior we want to test and we wanted to access the
localized strings.
But we realized it was nearly impossible so we decided to override some
localized strings in TestWebKitPlatformSupport::queryLocalizedString.
I think we can add WeekFormatTemplate there.

[Tiger, 2013-07-19 08:51:50]

> We have locale dependent behavior we want to test and we wanted to access the
> localized strings.
> But we realized it was nearly impossible so we decided to override some
> localized strings in TestWebKitPlatformSupport::queryLocalizedString.
> I think we can add WeekFormatTemplate there.

I've added an overload for this in queryLocalizedString now, and that seemed to
work just fine.

[jochen (gone - plz use gerrit), 2013-07-19 08:57:34]

lgtm

[commit-bot: I haz the power, 2013-07-19 09:14:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tiger@opera.com/18429012/17001

[commit-bot: I haz the power, 2013-07-19 09:28:44]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Tiger, 2013-07-19 11:10:29]

On 2013/07/19 09:28:44, I haz the power (commit-bot) wrote:
> Retried try job too often on chromium_presubmit for step(s) presubmit
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

Oh yeah, need someone with OWNER status to approve this I guess. Adding tkent.

[tkent, 2013-07-23 00:03:00]

Would you rebase the patch please?  Try bots failed to apply the patch.

[Tiger, 2013-08-05 09:48:51]

On 2013/07/23 00:03:00, tkent wrote:
> Would you rebase the patch please?  Try bots failed to apply the patch.

Back from vacation now -- rebased.

[commit-bot: I haz the power, 2013-08-05 09:50:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tiger@opera.com/18429012/31001

[commit-bot: I haz the power, 2013-08-05 10:00:07]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[tkent, 2013-08-05 22:28:57]

lgtm

[commit-bot: I haz the power, 2013-08-05 22:30:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tiger@opera.com/18429012/31001

[commit-bot: I haz the power, 2013-08-06 02:37:21]

Change committed as 215763