Enforce that strings are defined at compile time

[Michael Giuffrida]

TL;DR: Why do we #include "chromium_strings.h" in Chrome-branded builds?

We ran into an interesting issue where a string appeared in chromium_strings.grd but not in google_chrome_strings.grd [1].

As one would expect, the generated "chromium_strings.h" had the expected definition:

#define IDS_MY_STRING 123

but the generated "google_chrome_strings.h" did not.

However, using IDS_MY_STRING still compiles -- even in a Chrome-branded build. The string isn't in the resources pak for Google Chrome, so Chrome may crash when trying to use it.

This compiles because we always #include "chromium_strings.h", which had the #define as if the string was present, instead of "google_chrome_strings.h", which didn't have the #define.

Can we prevent this from happening again? Why don't we #include a common header that #includes the appropriate strings file depending on the build config?

[1] See CL fixing issue. The string actually was defined in both, but was inside an <if expr="chromeos"> block in google_chrome_strings.grd, so it wasn't defined in non-CrOS builds.

[Michael Giuffrida]

Coincidentally, I ran into a similar issue where the Google Chrome version of the string had incorrect placeholder values. This would have been detected by a DCHECK in the CQ if it had occurred in the Chromium version of the string.

[Nico Weber]

Your proposal makes sense to me from a distance. Maybe try implementing it and see if you run into any issues?

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/e5c402b1-95c6-4043-9dcc-4be01d9eaa07%40chromium.org.