Bug 58747 – REGRESSION (r84094): Many tests failing assertions in WebCore::localizedString on Windows

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Jessie Berlin]

http://trac.webkit.org/changeset/84094

http://build.webkit.org/builders/Windows%20XP%20Debug%20%28Tests%29/builds/27726
http://build.webkit.org/results/Windows%20XP%20Debug%20(Tests)/r84095%20(27727)/results.html

Looking at the crash logs, I think this is a problem for not just Windows XP, but all Windows ports. Since it is trying to write to bbadbeef, it is probably the ASSERT(isMainThread()) that is getting hit in debug builds (where as all the other Windows test bots are Release).

I am not sure yet where it is being called from a non-main thread. We should consider rolling back this change if it isn’t fixed soon.

[Adam Roben (:aroben)]

Here's the assertion:

http://trac.webkit.org/browser/trunk/Source/WebCore/platform/win/LocalizedStringsWin.cpp?rev=84087#L82

    ASSERT_WITH_MESSAGE(result.get() != notFound, "could not find localizable string %s in bundle", key);

[Jessie Berlin]

(In reply to comment #1)
> Here's the assertion:
> 
> http://trac.webkit.org/browser/trunk/Source/WebCore/platform/win/LocalizedStringsWin.cpp?rev=84087#L82
> 
>     ASSERT_WITH_MESSAGE(result.get() != notFound, "could not find localizable string %s in bundle", key);

The assertion failure seems to be happening under places like WebCore::imageTitle. Wouldn't it be expected to not find all image titles localized in the webkit bundle?

Maybe that assertion needs to removed and replaced with:

if (result.get() != notFound)
    return result.get();

and otherwise let it fall through to the non-cf version? (which just returns the string itself?)

Or maybe we shouldn't be trying to localize image titles at all?

[Adam Roben (:aroben)]

(In reply to comment #2)
> Wouldn't it be expected to not find all image titles localized in the webkit bundle?

Yes. But we aren't trying to find all image titles. We're trying to find a format string: "%@ $@x$@ pixels".

[Adam Roben (:aroben)]

I had some typos in that format string, but you get the idea.

[Timothy Hatcher]

Created attachment 90047 [details]
Proposed Change to fix the update-webkit-localizable-strings script

[WebKit Review Bot]

Attachment 90047 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Tools/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebKit/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 3 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Timothy Hatcher]

Comment on attachment 90047 [details]
Proposed Change to fix the update-webkit-localizable-strings script

Landed in r84158. Build fix in r84159.

[Jessie Berlin]

Fixed in http://trac.webkit.org/changeset/84192 by copying over the proper Localizable.strings file.