Probably asking a silly question, but would changing the about:flags messages (which contribute a lot to the not-translating bunch) to just static strings be viable?

We would need to change FeatureEntry, and about_flags.cc might end up with more #includes depending on where the strings end up being declared. But I could not figure out a good reason for having the descriptions in resources if we don't need translation.

Actually, yeah, I see no reason to not inline them either!

Issue 703121 has been merged into this issue.

Thanks for response in #2. I put together a proposal to migrate to plain const char arrays: https://docs.google.com/document/d/1n71OibFcBTlOdQqlLWF_Ek2CPXIKzZB-4vWTE8cfq8w/edit?usp=sharing

I will send it to chromium-dev for comments. If nobody raises significant concerns, I will attempt to implement that proposal.

I have a plan, nobody objects and I will start on that shortly.

Progress update:

The main CL https://codereview.chromium.org/2774203002/ is waiting for trybots, before I send it to review. There are two pre-requisite clean-ups waiting for review already: https://codereview.chromium.org/2774153002/ and https://codereview.chromium.org/2773323002/.

The main CL was done semi-automatically: I used the attached script to generate the new header and .cc file and to generate input for tools/git/mffr.py which did the renaming inside about_flags.cc and generated_resources.grd. I also used clang-format (which is now thankfully mandatory in src/). The manual part was checking that things make sense, changing flags_ui::FeatureEntry from IDs to const char*, and deleting a couple of no longer used resources.

The script attached to #6 was outdated, but the design doc has a copy inside https://docs.google.com/document/d/1n71OibFcBTlOdQqlLWF_Ek2CPXIKzZB-4vWTE8cfq8w/edit#heading=h.m6ziui902qg0

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0215a8ef53b8eb8faa446366db61113e6f5941a2

commit 0215a8ef53b8eb8faa446366db61113e6f5941a2
Author: vabr <vabr@chromium.org>
Date: Tue Mar 28 12:47:34 2017

Migrate about:flags messages to const char*

About:flags messages are currently non-translated resource strings. This brings some challenges, in particular, despite not being translated, these messages are copied in every language pack. There is no apparent reason to keep them as resources, and thus this CL changes them to normal const char* string literals.

Some unused resource strings are also dropped.

More details are in https://docs.google.com/document/d/1n71OibFcBTlOdQqlLWF_Ek2CPXIKzZB-4vWTE8cfq8w/edit#

BUG= 703134 

Review-Url: https://codereview.chromium.org/2774203002
Cr-Commit-Position: refs/heads/master@{#460072}

[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/chrome/app/chromeos_strings.grdp
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/chrome/app/generated_resources.grd
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/chrome/browser/BUILD.gn
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/chrome/browser/OWNERS
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/chrome/browser/about_flags.cc
[add] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/chrome/browser/flag_descriptions.cc
[add] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/components/flags_ui/feature_entry.cc
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/components/flags_ui/feature_entry.h
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/components/flags_ui/flags_state.cc
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/components/flags_ui/flags_state_unittest.cc
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/components/flags_ui_strings.grdp
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/ios/chrome/app/strings/ios_strings.grd
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/ios/chrome/browser/OWNERS
[modify] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/ios/chrome/browser/about_flags.mm
[add] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/ios/chrome/browser/ios_chrome_flag_descriptions.cc
[add] https://crrev.com/0215a8ef53b8eb8faa446366db61113e6f5941a2/ios/chrome/browser/ios_chrome_flag_descriptions.h

r460072 should complete the work on this. I will close the bug for now, but will of course reopen if the change causes troubles.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/47f8634a9cd73ded68ee3e5f2c0de85a868396b0

commit 47f8634a9cd73ded68ee3e5f2c0de85a868396b0
Author: vabr <vabr@chromium.org>
Date: Wed Mar 29 08:13:30 2017

Fix typo in OWNERS for flag_descriptions

Turns out https://codereview.chromium.org/2774203002 landed with a typo in
OWNERS files, mentioning "flags" instead of "flag" in the flag description file
names.

This has been discovered and fixed for //chrome by isherman in
https://codereview.chromium.org/2784723002/. It also needs fixing in iOS. To
speed up the fix, I am uploading this CL with both the //chrome and //ios
correction.

BUG= 703134 
TBR=sdefresne@chromium.org

Review-Url: https://codereview.chromium.org/2782103002
Cr-Commit-Position: refs/heads/master@{#460325}

[modify] https://crrev.com/47f8634a9cd73ded68ee3e5f2c0de85a868396b0/chrome/browser/OWNERS
[modify] https://crrev.com/47f8634a9cd73ded68ee3e5f2c0de85a868396b0/ios/chrome/browser/OWNERS

Would unduplicating the remaining non-translated strings still be worth it? Any idea how much we would save with that?

In chrome/app/generated_resources.grd I only see 19 instances of 'translateable="false"'. Counting 30 bytes per message and about 50 languages, this is roughly 30kB.

There are more resource files with translateable="false" entries. I did not count, but it could be a handful of hundreds. That might total up to about 1MB of savings. The fact that they are scattered over many files would mean more manual work to get it done, though.

Given #13, I'm going to re-open to track de-duping the remaining messages (or moving them likewise into c++.

Taking this one assuming no one else is working on it.

A cursor search in internal repos reveals between 100-200+ strings, albeit a lot of short strings, that are translateable="false". I'll work on removing them all and adding a presubmit to prevent new ones from being added.

A couple android non-translatable strings for 16kb in review: https://chromium-review.googlesource.com/c/521282/, potentially a couple more if we figure out something the ones used in xml, maybe ~12kb.

A couple native strings that can be converted to string consts, and all of security_state_strings.grdp for ~80kb. Assuming we are willing to use string formatting, I can go ahead and work on these.

Any objections to plain C++ string formatting to fill in non-translatable strings? Since they aren't being translated anyways, there's no loss of order.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1c0ac28d4278906bdcbe0492bfa34ca55e6e1e8a

commit 1c0ac28d4278906bdcbe0492bfa34ca55e6e1e8a
Author: Peter Wen <wnwen@google.com>
Date: Mon Jun 05 17:16:00 2017

Android: Remove non-translateable strings in clank

Despite not being translated, translateable="false" strings are still
copied into every language pack. Since they are not translateable, there
is no benefit to bloating size and keeping them as @strings rather than
string literals which proguard can inline.

Estimated size savings: 11kb

Bug:  703134 
Change-Id: I02d47eb221f50c734fcc7e0eac5833f18e3233d4
Reviewed-on: https://chromium-review.googlesource.com/521282
Commit-Queue: Peter Wen <wnwen@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477007}
[add] https://crrev.com/1c0ac28d4278906bdcbe0492bfa34ca55e6e1e8a/chrome/android/java/src/org/chromium/chrome/browser/ChromeStringConstants.java
[modify] https://crrev.com/1c0ac28d4278906bdcbe0492bfa34ca55e6e1e8a/chrome/android/java/src/org/chromium/chrome/browser/preferences/autofill/AutofillServerCardEditor.java
[modify] https://crrev.com/1c0ac28d4278906bdcbe0492bfa34ca55e6e1e8a/chrome/android/java/src/org/chromium/chrome/browser/preferences/autofill/AutofillServerProfilePreferences.java
[modify] https://crrev.com/1c0ac28d4278906bdcbe0492bfa34ca55e6e1e8a/chrome/android/java/src/org/chromium/chrome/browser/sync/ui/PassphraseDialogFragment.java
[modify] https://crrev.com/1c0ac28d4278906bdcbe0492bfa34ca55e6e1e8a/chrome/android/java/src/org/chromium/chrome/browser/sync/ui/PassphraseTypeDialogFragment.java
[modify] https://crrev.com/1c0ac28d4278906bdcbe0492bfa34ca55e6e1e8a/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/1c0ac28d4278906bdcbe0492bfa34ca55e6e1e8a/chrome/android/java_sources.gni

Chatted with agrieve@ offline, string formatting should have no performance impact, and decrease binary size. Starting to work on converting C++ non-translatable strings.

Targeting M-61 for Chrome Go.

+estark@, felt@ for migrating security_state_strings.grdp to plain C++ strings in case they intended it to be translated in the future.

Please let me know if you have concerns, I will wait on implementation.

+lgarron who has plans regarding security_state_strings.grdp: are you planning to translate these strings in the near future?

Yes, we are planning to translate them in the near future, and if there are untranslateable strings left I will try to `if` them out of Android: https://docs.google.com/document/d/1oYEKqoL-tKC1EwOmoYrT5TCBttQ9eCK9q2TfyC8uneA/edit#heading=h.au203bjfkch0

It might require some #ifdef in the C++ switch statements, though.

Okay, thanks for the update. Please do #ifdef them out of android as we are working towards Chrome Go (http://go/chrome-go-apk-size).

I will not inline those strings then.

In the meantime, will investigate into fallback to en-US for non-translatable strings.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f195282c45b356dfb1283ea9459e98cdb953b5bb

commit f195282c45b356dfb1283ea9459e98cdb953b5bb
Author: Peter Wen <wnwen@google.com>
Date: Wed Jun 07 19:43:32 2017

Move non-translatable strings to code.

Despite not being translated, translateable="false" strings are still
copied into every language pack. Since they are not translateable, there
is no benefit to bloating language pack size.

This CL is only for components/security_interstitial to keep it small.

Estimated savings: 6kb

Bug:  703134 
Change-Id: I04bb937710ea44631e08075dde9c4b7560c21060
Reviewed-on: https://chromium-review.googlesource.com/524552
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477731}
[modify] https://crrev.com/f195282c45b356dfb1283ea9459e98cdb953b5bb/chrome/browser/ui/views/download/download_feedback_dialog_view.cc
[modify] https://crrev.com/f195282c45b356dfb1283ea9459e98cdb953b5bb/components/security_interstitials/core/BUILD.gn
[modify] https://crrev.com/f195282c45b356dfb1283ea9459e98cdb953b5bb/components/security_interstitials/core/controller_client.cc
[add] https://crrev.com/f195282c45b356dfb1283ea9459e98cdb953b5bb/components/security_interstitials/core/urls.cc
[add] https://crrev.com/f195282c45b356dfb1283ea9459e98cdb953b5bb/components/security_interstitials/core/urls.h
[modify] https://crrev.com/f195282c45b356dfb1283ea9459e98cdb953b5bb/components/security_interstitials_strings.grdp

So, we have an interesting situation on Android for Issue 657299: we have a set of strings that we either need to send to DevTools (which is English-only) or show in native Android UI (translated). It would be preferable to maintain a single copy of the English version of each string, or at least store the translateable and non-translateable version next to each other.

Who is the best person to talk to about this situation?

How interesting indeed :). How large are the set of strings? It's certainly possible to change the code to allow one-off loading from a hard-coded en-US.pak, but if it's only a small number of strings, it might be easier to inline them into the C++ somehow.

On the order of a dozen or two, but adding a string every once in a while.

Inlining in C++ would be the least desirable, since that places the translatable and non-translatable sources far apart.

From your response, I gather that there isn't existing code to support loading a given string from en-US. Sounds like the best straightforward solution is to just have two copies of each string?

~20 strings turns into a significant overhead within .pak files, because even when not translated, it will be copied into ~50 different files. I think options are:

1. Custom build step that generates a .h/.cc file with the constants based on the values from the .grd
2. Change resource_bundle.cc to support accessing files from en-US.pak even when other languages are active.

#2 is a bit iffy on Android because we currently do not extract en-US.pak for non-english locales. If we were to add c++ logic for fetching english-only strings, we'd end up mmapping an entire additional .pak file, as well as have it extracted upon application start.

I'd recommend having a copy as c++ constants, but as you pointed out, don't actually copy & paste, write a simple script to generate the copy.

I see the following goals here, by my personal preferred priority:

1. If two source copies of a string are necessary, put them next to each other.
2. Avoid making 50 copies of a non-translateable string.
3. Avoid having two source copies of each string.

For now, I think I'll satisfy #1 stick with two copies in the `grd` just so I can get a working CL. (I can ifdef the translated half of strings out of desktop if desired, though.)

Would you or someone else be able to write a system like you mentioned so that we can satisfy all 3? I don't have time to spend on this myself, but I'm happy to call a special function and/or split the special strings out of page_info_strings.grd .

For prototype, I think two copies in the .grd is fine, but note that if you commit >10 strings, you're likely to cause a perf alert to fire, and you'll be asked to shrink the binary size impact of your change. Best thing to do is to use //tools/binary_size/diagnose_bloat.py on your commit locally to see what the overhead is.

I'm happy to do reviews, but also don't have time to do the implementation.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eff78bd0568fb325c89ca9b47c00b30d8fc5f3e4

commit eff78bd0568fb325c89ca9b47c00b30d8fc5f3e4
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Jul 20 01:29:28 2017

android_chrome_strings.grd: Insert locale at runtime rather for URLs

Saves 8904 bytes in resources.arsc

BUG= 703134 

Change-Id: Id82597710ab1055a083bb354cf0feeeea6f40be6
Reviewed-on: https://chromium-review.googlesource.com/578389
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488080}
[modify] https://crrev.com/eff78bd0568fb325c89ca9b47c00b30d8fc5f3e4/chrome/android/java/src/org/chromium/chrome/browser/firstrun/FirstRunActivity.java
[modify] https://crrev.com/eff78bd0568fb325c89ca9b47c00b30d8fc5f3e4/chrome/android/java/src/org/chromium/chrome/browser/firstrun/LightweightFirstRunActivity.java
[modify] https://crrev.com/eff78bd0568fb325c89ca9b47c00b30d8fc5f3e4/chrome/android/java/src/org/chromium/chrome/browser/preferences/HyperlinkPreference.java
[modify] https://crrev.com/eff78bd0568fb325c89ca9b47c00b30d8fc5f3e4/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/eff78bd0568fb325c89ca9b47c00b30d8fc5f3e4/ui/android/java/src/org/chromium/ui/base/LocalizationUtils.java

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c896a653848031478c06d65d6882f14d2ab2e3c2

commit c896a653848031478c06d65d6882f14d2ab2e3c2
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Jul 21 14:49:31 2017

Convert IDS_AUTOFILL_SHOW_PREDICTIONS_TITLE to an inline string

Having non-translated strings in .pak files is quite inefficient since
they are copied into each locale .pak file.

Changing this one string decreases apk size by 3,641 bytes.

BUG= 703134 

Change-Id: I7b64c2d7a48544a7e780e3c300044fdba33b3920
Reviewed-on: https://chromium-review.googlesource.com/578214
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488655}
[modify] https://crrev.com/c896a653848031478c06d65d6882f14d2ab2e3c2/components/autofill/content/renderer/form_cache.cc
[modify] https://crrev.com/c896a653848031478c06d65d6882f14d2ab2e3c2/components/autofill_strings.grdp

I think the last of the existing translatable=false strings for Android has now be moved to c++ / java. The only ones left are those related to bug 657299, which I think are just translatable=false because the feature isn't launched yet.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a70430404ca23ed7b4e35bad58148982fac7b970

commit a70430404ca23ed7b4e35bad58148982fac7b970
Author: Roger McFarlane <rogerm@chromium.org>
Date: Thu Aug 03 21:34:08 2017

[autofill] Fix --show-autofill-type-predictions.

Corrects the code that generates field title strings when the
--show-autofill-type-predictions flag is specified. Formatting
bugs were introduced when the string was moved out of the .pak
file and into code (escape sequences for string replacement
were retained and some values were duplicated/missing due to
copy-paste).

BUG= 703134 

Change-Id: Id81a974be4ee12f0c6e2870da2b80c592962d730
Reviewed-on: https://chromium-review.googlesource.com/600994
Commit-Queue: Roger McFarlane <rogerm@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491852}
[modify] https://crrev.com/a70430404ca23ed7b4e35bad58148982fac7b970/components/autofill/content/renderer/form_cache.cc