Migrate about:flags messages to const char*

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}
Committed: https://chromium.googlesource.com/chromium/src/+/0215a8ef53b8eb8faa446366db61113e6f5941a2

[vabr (Chromium), 2017-03-26 04:47:19]

Description was changed from

==========
Migrate about:flags messages to const char*

About:flags messages are currently a 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.

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

BUG=703134
==========

to

==========
Migrate about:flags messages to const char*

About:flags messages are currently a 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-4vWTE8cfq...

BUG=703134
==========

[vabr (Chromium), 2017-03-26 05:29:54]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-26 05:30:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-26 05:38:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-26 05:38:00]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[vabr (Chromium), 2017-03-26 06:10:05]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-26 06:10:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-26 06:24:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-26 06:24:47]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[vabr (Chromium), 2017-03-26 07:14:28]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-26 07:14:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-26 07:41:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-26 07:41:52]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[vabr (Chromium), 2017-03-26 08:17:04]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-26 08:17:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vabr (Chromium), 2017-03-26 08:17:37]

Description was changed from

==========
Migrate about:flags messages to const char*

About:flags messages are currently a 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-4vWTE8cfq...

BUG=703134
==========

to

==========
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-4vWTE8cfq...

BUG=703134
==========

[commit-bot: I haz the power, 2017-03-26 10:01:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-26 10:01:40]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2017-03-26 13:27:43]

vabr@chromium.org changed reviewers:
+ agrieve@chromium.org, asvitkine@chromium.org, jochen@chromium.org,
sdefresne@chromium.org

[vabr (Chromium), 2017-03-26 13:27:45]

Hi all,

Please review as follows:

asvitkine@: components/flags_ui/*

agrieve@: chrome/browser/about_flags.cc, chrome/browser/BUILD.gn, chrome/app

sdefresne@: ios/* and components/flags_ui_strings.grdp

jochen@: chrome/browser/OWNERS, chrome/browser/flag_descriptions.*

Thanks!
Vaclav

https://codereview.chromium.org/2774203002/diff/160001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/app/generated_r...
chrome/app/generated_resources.grd:5027: +      <message
name="IDS_PASSWORD_MANAGER_ACCOUNT_CHOOSER_TITLE_MANY_ACCOUNTS" desc="The title
of the account chooser dialog/infobar for more than one account.">
I'm not sure why the diff looks like I have added new stuff, it seems to be just
randomly picking a particular "</message>" line to "synchronize" on below.
The only thing I did to this file is remove IDS_FLAGS_* messages, the rest has
been there already.

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/OWNERS
File chrome/browser/OWNERS (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/OWNERS#...
chrome/browser/OWNERS:70: per-file flags_descriptions.*=*
This is because the equivalent information was in files with the same broad
ownership.

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
File chrome/browser/flags_descriptions.h (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
chrome/browser/flags_descriptions.h:1: // Copyright 2017 The Chromium Authors.
All rights reserved.
The strings / comments in this file are copied from the resources in the way
described in the linked design doc. I made no attempt to fix typos etc. in them,
to keep the churn low (it is 700+ messages).

https://codereview.chromium.org/2774203002/diff/160001/ios/chrome/browser/OWNERS
File ios/chrome/browser/OWNERS (right):

https://codereview.chromium.org/2774203002/diff/160001/ios/chrome/browser/OWN...
ios/chrome/browser/OWNERS:6: per-file ios_chrome_flags_descriptions.*=*
This is because the equivalent information was in files with the same broad
ownership.

[vabr (Chromium), 2017-03-26 13:27:55]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-26 13:28:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-26 15:27:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-26 15:27:05]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jochen (gone - plz use gerrit), 2017-03-27 12:38:52]

lgtm

[agrieve, 2017-03-27 15:43:24]

lgtm

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.g...
chrome/browser/BUILD.gn:410: "flags_descriptions.h",
Super nit: "flag_descriptions.*" would sound better.

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
File chrome/browser/flags_descriptions.cc (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
chrome/browser/flags_descriptions.cc:7: namespace flags_descriptions {
likewise for name nit: flag_descriptions

[Alexei Svitkine (slow), 2017-03-27 16:37:29]

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.g...
chrome/browser/BUILD.gn:410: "flags_descriptions.h",
On 2017/03/27 15:43:24, agrieve wrote:
> Super nit: "flag_descriptions.*" would sound better.

We're not doing it for the other source files... so I think we should keep
consistency unless we decide to change all of the entries.

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
File chrome/browser/flags_descriptions.cc (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
chrome/browser/flags_descriptions.cc:12: R"*(Enable Material Design
bookmarks)*";
I don't think we should be prefixing all of these with R. It just adds extra
noise to vast majority of these. Suggest adding R (in your script) only if the
content has something that needs it - e.g. an escape character \ or inner "'s

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
File chrome/browser/flags_descriptions.h (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
chrome/browser/flags_descriptions.h:10: #include "media/media_features.h"
Nit: Maybe add a comment why this is needed?

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
chrome/browser/flags_descriptions.h:11: 
Please add a file-level comment here about what this file is for.

https://codereview.chromium.org/2774203002/diff/160001/components/flags_ui/fl...
File components/flags_ui/flags_state.cc (right):

https://codereview.chromium.org/2774203002/diff/160001/components/flags_ui/fl...
components/flags_ui/flags_state.cc:514:
base::UTF8ToUTF16(base::StringPiece(entry.visible_name)));
Looks like SetString() accepts a StringPiece as the second param:

https://cs.chromium.org/chromium/src/base/values.h?rcl=0e44ebc7dcf7c343d45694...

So no need to do our own UTF8ToUTF16 here. Same below.

[vabr (Chromium), 2017-03-27 16:48:26]

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.g...
chrome/browser/BUILD.gn:410: "flags_descriptions.h",
On 2017/03/27 16:37:28, Alexei Svitkine (slow) wrote:
> On 2017/03/27 15:43:24, agrieve wrote:
> > Super nit: "flag_descriptions.*" would sound better.
> 
> We're not doing it for the other source files... so I think we should keep
> consistency unless we decide to change all of the entries.

I think agrieve@ meant I keep "flags" singular: "flag_descriptions", not that I
should use '*' instead of file endings.

[Alexei Svitkine (slow), 2017-03-27 16:55:39]

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.g...
chrome/browser/BUILD.gn:410: "flags_descriptions.h",
On 2017/03/27 16:48:25, vabr (Chromium) wrote:
> On 2017/03/27 16:37:28, Alexei Svitkine (slow) wrote:
> > On 2017/03/27 15:43:24, agrieve wrote:
> > > Super nit: "flag_descriptions.*" would sound better.
> > 
> > We're not doing it for the other source files... so I think we should keep
> > consistency unless we decide to change all of the entries.
> 
> I think agrieve@ meant I keep "flags" singular: "flag_descriptions", not that
I
> should use '*' instead of file endings.

Oops, I did indeed misread that comment. :)

[vabr (Chromium), 2017-03-27 18:11:30]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 18:12:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vabr (Chromium), 2017-03-27 18:12:33]

Thanks everybody for the review so far!

Alexei and agrieve@, please have a look at how I addressed your comments.

sdefresne@, please review ios/* and components/flags_ui_strings.grdp

Thank you all!
Vaclav

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.g...
chrome/browser/BUILD.gn:410: "flags_descriptions.h",
On 2017/03/27 15:43:24, agrieve wrote:
> Super nit: "flag_descriptions.*" would sound better.

Done.

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
File chrome/browser/flags_descriptions.cc (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
chrome/browser/flags_descriptions.cc:7: namespace flags_descriptions {
On 2017/03/27 15:43:24, agrieve wrote:
> likewise for name nit: flag_descriptions

Done.

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
chrome/browser/flags_descriptions.cc:12: R"*(Enable Material Design
bookmarks)*";
On 2017/03/27 16:37:28, Alexei Svitkine (slow) wrote:
> I don't think we should be prefixing all of these with R. It just adds extra
> noise to vast majority of these. Suggest adding R (in your script) only if the
> content has something that needs it - e.g. an escape character \ or inner "'s

Done.

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
File chrome/browser/flags_descriptions.h (right):

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
chrome/browser/flags_descriptions.h:10: #include "media/media_features.h"
On 2017/03/27 16:37:29, Alexei Svitkine (slow) wrote:
> Nit: Maybe add a comment why this is needed?

Done.

https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
chrome/browser/flags_descriptions.h:11: 
On 2017/03/27 16:37:29, Alexei Svitkine (slow) wrote:
> Please add a file-level comment here about what this file is for.

Done.

https://codereview.chromium.org/2774203002/diff/160001/components/flags_ui/fl...
File components/flags_ui/flags_state.cc (right):

https://codereview.chromium.org/2774203002/diff/160001/components/flags_ui/fl...
components/flags_ui/flags_state.cc:514:
base::UTF8ToUTF16(base::StringPiece(entry.visible_name)));
On 2017/03/27 16:37:29, Alexei Svitkine (slow) wrote:
> Looks like SetString() accepts a StringPiece as the second param:
> 
>
https://cs.chromium.org/chromium/src/base/values.h?rcl=0e44ebc7dcf7c343d45694...
> 
> So no need to do our own UTF8ToUTF16 here. Same below.

Thanks for catching this, it would have been indeed a silly thing to do, with
base::Value actually storing the 8-byte string.

[agrieve, 2017-03-27 18:53:16]

On 2017/03/27 18:12:33, vabr (Chromium) wrote:
> Thanks everybody for the review so far!
> 
> Alexei and agrieve@, please have a look at how I addressed your comments.
> 
> sdefresne@, please review ios/* and components/flags_ui_strings.grdp
> 
> Thank you all!
> Vaclav
> 
> https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.gn
> File chrome/browser/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/BUILD.g...
> chrome/browser/BUILD.gn:410: "flags_descriptions.h",
> On 2017/03/27 15:43:24, agrieve wrote:
> > Super nit: "flag_descriptions.*" would sound better.
> 
> Done.
> 
>
https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
> File chrome/browser/flags_descriptions.cc (right):
> 
>
https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
> chrome/browser/flags_descriptions.cc:7: namespace flags_descriptions {
> On 2017/03/27 15:43:24, agrieve wrote:
> > likewise for name nit: flag_descriptions
> 
> Done.
> 
>
https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
> chrome/browser/flags_descriptions.cc:12: R"*(Enable Material Design
> bookmarks)*";
> On 2017/03/27 16:37:28, Alexei Svitkine (slow) wrote:
> > I don't think we should be prefixing all of these with R. It just adds extra
> > noise to vast majority of these. Suggest adding R (in your script) only if
the
> > content has something that needs it - e.g. an escape character \ or inner
"'s
> 
> Done.
> 
>
https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
> File chrome/browser/flags_descriptions.h (right):
> 
>
https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
> chrome/browser/flags_descriptions.h:10: #include "media/media_features.h"
> On 2017/03/27 16:37:29, Alexei Svitkine (slow) wrote:
> > Nit: Maybe add a comment why this is needed?
> 
> Done.
> 
>
https://codereview.chromium.org/2774203002/diff/160001/chrome/browser/flags_d...
> chrome/browser/flags_descriptions.h:11: 
> On 2017/03/27 16:37:29, Alexei Svitkine (slow) wrote:
> > Please add a file-level comment here about what this file is for.
> 
> Done.
> 
>
https://codereview.chromium.org/2774203002/diff/160001/components/flags_ui/fl...
> File components/flags_ui/flags_state.cc (right):
> 
>
https://codereview.chromium.org/2774203002/diff/160001/components/flags_ui/fl...
> components/flags_ui/flags_state.cc:514:
> base::UTF8ToUTF16(base::StringPiece(entry.visible_name)));
> On 2017/03/27 16:37:29, Alexei Svitkine (slow) wrote:
> > Looks like SetString() accepts a StringPiece as the second param:
> > 
> >
>
https://cs.chromium.org/chromium/src/base/values.h?rcl=0e44ebc7dcf7c343d45694...
> > 
> > So no need to do our own UTF8ToUTF16 here. Same below.
> 
> Thanks for catching this, it would have been indeed a silly thing to do, with
> base::Value actually storing the 8-byte string.

lgtm

[Alexei Svitkine (slow), 2017-03-27 18:56:50]

lgtm once you address the important comment below

https://codereview.chromium.org/2774203002/diff/180001/chrome/browser/flag_de...
File chrome/browser/flag_descriptions.cc (right):

https://codereview.chromium.org/2774203002/diff/180001/chrome/browser/flag_de...
chrome/browser/flag_descriptions.cc:15: "If enabled, the chrome://bookmarks/ URL
loads the Material Design"
Are you missing a space between the end of this line and start of next line?

(Fix throughout)

[commit-bot: I haz the power, 2017-03-27 19:40:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-27 19:40:30]

Dry run: This issue passed the CQ dry run.

[vabr (Chromium), 2017-03-27 20:04:54]

The CQ bit was checked by vabr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 20:05:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[vabr (Chromium), 2017-03-27 20:12:56]

Thanks!

Once sdefresne@ has a chance to review ios/* and
components/flags_ui_strings.grdp and approves, I will send this to CQ.

Cheers,
Vaclav

https://codereview.chromium.org/2774203002/diff/180001/chrome/browser/flag_de...
File chrome/browser/flag_descriptions.cc (right):

https://codereview.chromium.org/2774203002/diff/180001/chrome/browser/flag_de...
chrome/browser/flag_descriptions.cc:15: "If enabled, the chrome://bookmarks/ URL
loads the Material Design"
On 2017/03/27 18:56:49, Alexei Svitkine (slow) wrote:
> Are you missing a space between the end of this line and start of next line?
> 
> (Fix throughout)

Oops, thanks for catching that! While it would save about a kB of binary size
:), it's much better with the spaces.
Done.

[commit-bot: I haz the power, 2017-03-27 21:28:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-27 21:28:18]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2017-03-28 09:42:28]

components/flags_ui_strings.grdp & ios/* lgtm

https://codereview.chromium.org/2774203002/diff/200001/ios/chrome/browser/ios...
File ios/chrome/browser/ios_chrome_flag_descriptions.h (right):

https://codereview.chromium.org/2774203002/diff/200001/ios/chrome/browser/ios...
ios/chrome/browser/ios_chrome_flag_descriptions.h:14: // Name of about:flag
option to control redirection to the task scheduler.
nit, may be done in a followup: for consistency with the other flags, can you
change this to the following (and sort it before
kBrowserTaskSchedulerDescription, here and in the implementation file)?

// Title for the flag to control redirection to the task scheduler.
extern const char kBrowserTaskScheduler;

[vabr (Chromium), 2017-03-28 10:10:31]

Thanks!

Sending to CQ now.

Cheers,
Vaclav

https://codereview.chromium.org/2774203002/diff/200001/ios/chrome/browser/ios...
File ios/chrome/browser/ios_chrome_flag_descriptions.h (right):

https://codereview.chromium.org/2774203002/diff/200001/ios/chrome/browser/ios...
ios/chrome/browser/ios_chrome_flag_descriptions.h:14: // Name of about:flag
option to control redirection to the task scheduler.
On 2017/03/28 09:42:28, sdefresne wrote:
> nit, may be done in a followup: for consistency with the other flags, can you
> change this to the following (and sort it before
> kBrowserTaskSchedulerDescription, here and in the implementation file)?
> 
> // Title for the flag to control redirection to the task scheduler.
> extern const char kBrowserTaskScheduler;

Done.

[vabr (Chromium), 2017-03-28 10:10:43]

The CQ bit was checked by vabr@chromium.org

[vabr (Chromium), 2017-03-28 10:10:44]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
asvitkine@chromium.org, agrieve@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2774203002/#ps220001
(title: "kBrowserTaskScheduler")

[commit-bot: I haz the power, 2017-03-28 10:10:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 10:12:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 10:12:49]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[vabr (Chromium), 2017-03-28 10:14:18]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2017-03-28 10:14:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 10:19:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-28 10:19:39]

Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[vabr (Chromium), 2017-03-28 10:26:44]

The CQ bit was checked by vabr@chromium.org

[vabr (Chromium), 2017-03-28 10:26:45]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
asvitkine@chromium.org, agrieve@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2774203002/#ps240001
(title: "fix iOS compilation")

[commit-bot: I haz the power, 2017-03-28 10:26:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sdefresne, 2017-03-28 11:46:25]

lgtm

https://codereview.chromium.org/2774203002/diff/200001/ios/chrome/browser/ios...
File ios/chrome/browser/ios_chrome_flag_descriptions.h (right):

https://codereview.chromium.org/2774203002/diff/200001/ios/chrome/browser/ios...
ios/chrome/browser/ios_chrome_flag_descriptions.h:14: // Name of about:flag
option to control redirection to the task scheduler.
On 2017/03/28 10:10:31, vabr (Chromium) wrote:
> On 2017/03/28 09:42:28, sdefresne wrote:
> > nit, may be done in a followup: for consistency with the other flags, can
you
> > change this to the following (and sort it before
> > kBrowserTaskSchedulerDescription, here and in the implementation file)?
> > 
> > // Title for the flag to control redirection to the task scheduler.
> > extern const char kBrowserTaskScheduler;
> 
> Done.

Ooops sorry, I forgot "[]" which broke the build :-/

[vabr (Chromium), 2017-03-28 11:50:07]

On 2017/03/28 11:46:25, sdefresne wrote:
> lgtm
> 
>
https://codereview.chromium.org/2774203002/diff/200001/ios/chrome/browser/ios...
> File ios/chrome/browser/ios_chrome_flag_descriptions.h (right):
> 
>
https://codereview.chromium.org/2774203002/diff/200001/ios/chrome/browser/ios...
> ios/chrome/browser/ios_chrome_flag_descriptions.h:14: // Name of about:flag
> option to control redirection to the task scheduler.
> On 2017/03/28 10:10:31, vabr (Chromium) wrote:
> > On 2017/03/28 09:42:28, sdefresne wrote:
> > > nit, may be done in a followup: for consistency with the other flags, can
> you
> > > change this to the following (and sort it before
> > > kBrowserTaskSchedulerDescription, here and in the implementation file)?
> > > 
> > > // Title for the flag to control redirection to the task scheduler.
> > > extern const char kBrowserTaskScheduler;
> > 
> > Done.
> 
> Ooops sorry, I forgot "[]" which broke the build :-/

I should have double-checked before copy pasting into our repository! :)

[commit-bot: I haz the power, 2017-03-28 12:47:39]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1490696804520190,
"parent_rev": "5376139411f2b362d24abc27425f3827697a46d4", "commit_rev":
"0215a8ef53b8eb8faa446366db61113e6f5941a2"}

[commit-bot: I haz the power, 2017-03-28 12:48:28]

Description was changed from

==========
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-4vWTE8cfq...

BUG=703134
==========

to

==========
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-4vWTE8cfq...

BUG=703134

Review-Url: https://codereview.chromium.org/2774203002
Cr-Commit-Position: refs/heads/master@{#460072}
Committed:
https://chromium.googlesource.com/chromium/src/+/0215a8ef53b8eb8faa446366db61...
==========

[commit-bot: I haz the power, 2017-03-28 12:48:31]

Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/0215a8ef53b8eb8faa446366db61...