Move strings from webkit/ to content/.

Move strings from webkit/ content/.

This moves the strings that were previously in webkit/glue/resources/*.xtb into
 content/app/strings/translations/*.xtb and makes the necessary GYP, GN and C++ changes
 to make all this work.

BUG=338338
TEST=content_shell, content_unittests. No functional changes.
R=jochen@chromium.org,avi@chromium.org
TBR=darin@chromium.org

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

[tfarina, 2014-07-27 22:48:29]

Jochen -> everything
Boliu -> android_webview change

[boliu, 2014-07-27 23:12:31]

android_webview lgtm

cc some webview folks fyi

[mkosiba (inactive), 2014-07-28 08:57:38]

On 2014/07/27 23:12:31, boliu wrote:
> android_webview lgtm
> 
> cc some webview folks fyi

android_webview lgtm

[jochen (gone - plz use gerrit), 2014-07-28 08:57:46]

On 2014/07/27 at 23:12:31, boliu wrote:
> android_webview lgtm
> 
> cc some webview folks fyi

as john mentioned, please move the strings to below content/app (similar to
where chrome/app has strings) instead of adding a new content subdir

[tfarina, 2014-07-30 22:26:24]

John/Jochen, I moved into content/app/strings.

ptal!

[jam, 2014-07-31 22:25:01]

On 2014/07/30 22:26:24, tfarina wrote:
> John/Jochen, I moved into content/app/strings.
> 
> ptal!

I scanned quickly, and it looks good. I defer to jochen for full review (sorry
overloaded with CY)

[jochen (gone - plz use gerrit), 2014-08-01 07:52:16]

lgtm assuming the new build changes are indeed required

https://codereview.chromium.org/423613003/diff/230001/chrome/chrome_renderer....
File chrome/chrome_renderer.gypi (right):

https://codereview.chromium.org/423613003/diff/230001/chrome/chrome_renderer....
chrome/chrome_renderer.gypi:290:
'../content/app/strings/content_strings.gyp:content_strings',
why do you have to add this here, but not delete webkit_strings anywhere?

https://codereview.chromium.org/423613003/diff/230001/chrome/renderer/BUILD.gn
File chrome/renderer/BUILD.gn (right):

https://codereview.chromium.org/423613003/diff/230001/chrome/renderer/BUILD.g...
chrome/renderer/BUILD.gn:43: "//content/app/strings",
same question here?

https://codereview.chromium.org/423613003/diff/230001/components/components_t...
File components/components_tests.gyp (right):

https://codereview.chromium.org/423613003/diff/230001/components/components_t...
components/components_tests.gyp:846:
'<(DEPTH)/content/app/strings/content_strings.gyp:content_strings',
and here?

[tfarina, 2014-08-02 00:57:31]

https://codereview.chromium.org/423613003/diff/230001/chrome/chrome_renderer....
File chrome/chrome_renderer.gypi (right):

https://codereview.chromium.org/423613003/diff/230001/chrome/chrome_renderer....
chrome/chrome_renderer.gypi:290:
'../content/app/strings/content_strings.gyp:content_strings',
On 2014/08/01 07:52:16, jochen wrote:
> why do you have to add this here, but not delete webkit_strings anywhere?
they were necessary to fix trybot results.

[tfarina, 2014-08-03 14:16:57]

Nico, could you review the action rule?

Thank you for your patience.

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
File content/app/strings/content_strings.gyp (right):

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
content/app/strings/content_strings.gyp:23: 'action_name': 'rm_old_file',
is it fine to have it here?

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
content/app/strings/content_strings.gyp:28: '<(INTERMEDIATE_DIR)/dummy_stamp',
this is necessary, otherwise the ninja rule will build:

build :                 
    content_strings_target_rm_old_file_219febaf7ad770a6016bc4ac4c3e0d15 $       

    ../../build/whitespace_file.txt                                             


and when you run:

ninja -C out/Debug content_strings

ninja will complain with:
ninja: error: obj/content/app/strings/content_strings.ninja:224: expected path
build : content_strings_target_rm_old_file_219febaf7ad770a6016bc4ac4c3e0...
      ^ near here

[tfarina, 2014-08-03 14:17:33]

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
File content/app/strings/content_strings.gyp (right):

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
content/app/strings/content_strings.gyp:28: '<(INTERMEDIATE_DIR)/dummy_stamp',
On 2014/08/03 14:16:57, tfarina wrote:
> this is necessary, otherwise the ninja rule will build:
> 
s/will build/will be

[tfarina, 2014-08-03 14:30:12]

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
File content/app/strings/content_strings.gyp (right):

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
content/app/strings/content_strings.gyp:33: 'import os, sys;
os.remove(sys.argv[1])',
we also need os.path.exists(sys.argv[1]), otherwise in subsequent runs this will
try to remove a file that does not exist and will generate an error;

OSError: [Errno 2] No such file or directory:
'../../../out/Debug/gen/webkit/grit/webkit_strings.h'


But python didn't like the way I did:

FAILED: cd ../../content/app/strings; python -c "import os, sys; if
os.path.exists(sys.argv[1]): os.remove(sys.argv[1])"
../../../out/Debug/gen/webkit/grit/webkit_strings.h
  File "<string>", line 1
    import os, sys; if os.path.exists(sys.argv[1]): os.remove(sys.argv[1])
                     ^

Looks like I will have to put this in a separate py file?

[jochen (gone - plz use gerrit), 2014-08-04 08:32:38]

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
File content/app/strings/content_strings.gyp (right):

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
content/app/strings/content_strings.gyp:23: 'action_name': 'rm_old_file',
On 2014/08/03 14:16:57, tfarina wrote:
> is it fine to have it here?

i don't think we should have such magic in our gyp files.

Use a landmine for the bots and send a PSA.

[Nico, 2014-08-04 16:57:41]

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
File content/app/strings/content_strings.gyp (right):

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
content/app/strings/content_strings.gyp:23: 'action_name': 'rm_old_file',
On 2014/08/04 08:32:38, jochen wrote:
> On 2014/08/03 14:16:57, tfarina wrote:
> > is it fine to have it here?
> 
> i don't think we should have such magic in our gyp files.
> 
> Use a landmine for the bots and send a PSA.

How is a PSA better than things Just Working automatically?

(FWIW, I think the better way to make things Just Work is to make grit includes
fully qualified.)

[Ken Russell (switch to Gerrit), 2014-08-05 01:07:10]

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
File content/app/strings/content_strings.gyp (right):

https://codereview.chromium.org/423613003/diff/270001/content/app/strings/con...
content/app/strings/content_strings.gyp:23: 'action_name': 'rm_old_file',
On 2014/08/04 16:57:41, Nico (away) wrote:
> On 2014/08/04 08:32:38, jochen wrote:
> > On 2014/08/03 14:16:57, tfarina wrote:
> > > is it fine to have it here?
> > 
> > i don't think we should have such magic in our gyp files.
> > 
> > Use a landmine for the bots and send a PSA.
> 
> How is a PSA better than things Just Working automatically?
> 
> (FWIW, I think the better way to make things Just Work is to make grit
includes
> fully qualified.)

I think whatever magic is needed to make this work automatically, without a
manual clobber build, should be included in this CL. Developers like myself
switch between multiple machines, and coming back to a machine a week later,
doing an update and nearly-clean build, and then finding that a clobber build
was needed after all is a waste of time. These gyp targets can be removed in a
few months once the changes have cycled through everyone's workspaces.

[tfarina, 2014-08-05 14:06:30]

Nico, could you answer my question in content/app/strings/content_strings.gyp
about the if clause?

[jochen (gone - plz use gerrit), 2014-08-05 14:12:44]

I'd either make the existing magic work for developers (i.e. the landmines), or
fix grit.

[tfarina, 2014-08-05 14:19:41]

On Tue, Aug 5, 2014 at 11:12 AM, <jochen@chromium.org> wrote:

> I'd either make the existing magic work for developers (i.e. the
> landmines), or
> fix grit.
>
> Am I missing something, or are you guys requiring me to fix the
relative/full path issue?

-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jochen (gone - plz use gerrit), 2014-08-05 15:31:42]

I'd go for the landmines.


On Tue, Aug 5, 2014 at 4:19 PM, Thiago Farina <tfarina@chromium.org> wrote:

>
>
>
> On Tue, Aug 5, 2014 at 11:12 AM, <jochen@chromium.org> wrote:
>
>> I'd either make the existing magic work for developers (i.e. the
>> landmines), or
>> fix grit.
>>
>> Am I missing something, or are you guys requiring me to fix the
> relative/full path issue?
>
> --
> Thiago Farina
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[tfarina, 2014-08-05 16:29:14]

On Tue, Aug 5, 2014 at 12:31 PM, Jochen Eisinger <jochen@chromium.org>
wrote:

> I'd go for the landmines.
>
>
> Kenneth, Nico, is this fine by you?

I feel I'm being taken from one side to another without reaching nowhere.
Who is in position to take a final decision? landmines vs gyp/python rule.

I just want to move forward with this.

-- 
Thiago Farina

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2014-08-05 17:02:29]

On Tue, Aug 5, 2014 at 9:29 AM, Thiago Farina <tfarina@chromium.org> wrote:

>
>
>
> On Tue, Aug 5, 2014 at 12:31 PM, Jochen Eisinger <jochen@chromium.org>
> wrote:
>
>> I'd go for the landmines.
>>
>>
>> Kenneth, Nico, is this fine by you?
>
> I feel I'm being taken from one side to another without reaching nowhere.
> Who is in position to take a final decision? landmines vs gyp/python rule.
>

I'm happy with either. Having landmines for devs seems generally useful
(but I expect them to work less well for devs since some folks have
multiple output directories).


> I just want to move forward with this.
>
> --
> Thiago Farina
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ken Russell (switch to Gerrit), 2014-08-05 17:42:00]

On 2014/08/05 17:02:29, Nico (away) wrote:
> On Tue, Aug 5, 2014 at 9:29 AM, Thiago Farina <mailto:tfarina@chromium.org>
wrote:
> 
> >
> >
> >
> > On Tue, Aug 5, 2014 at 12:31 PM, Jochen Eisinger
<mailto:jochen@chromium.org>
> > wrote:
> >
> >> I'd go for the landmines.
> >>
> >>
> >> Kenneth, Nico, is this fine by you?
> >
> > I feel I'm being taken from one side to another without reaching nowhere.
> > Who is in position to take a final decision? landmines vs gyp/python rule.
> >
> 
> I'm happy with either. Having landmines for devs seems generally useful
> (but I expect them to work less well for devs since some folks have
> multiple output directories).

If the gyp rules you've added will avoid the need for a landmine, and avoid the
need for developers to manually clobber their builds, then I think you should
add them. But it is your decision - Nico and Jochen are both happy with either
approach.

All you really need at this point are the owners' thumbs-up. I'm not an owner in
any of these directories.

[tfarina, 2014-08-06 14:24:20]

Jochen, Nico, I added the python script and it is working.

ptal!

[tfarina, 2014-08-07 13:33:53]

Jochen, ping??

[jochen (gone - plz use gerrit), 2014-08-07 15:11:26]

On 2014/08/07 13:33:53, tfarina wrote:
> Jochen, ping??

Add I said before, I'd rather have a landmine and make developer build respect
them.

Once you remove this script, somebody will sync their old build, and it'll
break.

[Nico, 2014-08-07 15:14:09]

https://codereview.chromium.org/423613003/diff/310001/chrome/renderer/pepper/...
File chrome/renderer/pepper/pepper_pdf_host.cc (right):

https://codereview.chromium.org/423613003/diff/310001/chrome/renderer/pepper/...
chrome/renderer/pepper/pepper_pdf_host.cc:15: #include "grit/content_strings.h"
Can you use a qualified include for this? Then you probably don't need that
cleanup python script (like https://codereview.chromium.org/430083002/ – i
thought you were using blink_resources.h, hence me suggesting to wait for that
to go in)

[tfarina, 2014-08-07 15:59:28]

https://codereview.chromium.org/423613003/diff/310001/chrome/renderer/pepper/...
File chrome/renderer/pepper/pepper_pdf_host.cc (right):

https://codereview.chromium.org/423613003/diff/310001/chrome/renderer/pepper/...
chrome/renderer/pepper/pepper_pdf_host.cc:15: #include "grit/content_strings.h"
On 2014/08/07 15:14:09, Nico (very away) wrote:
> Can you use a qualified include for this?
I'm all for it. I will try this later tonight.

[tfarina, 2014-08-08 02:20:11]

Finally switched to use qualified path.

ptal!

[tfarina, 2014-08-08 22:22:40]

Avi, could you do the final review for content/?

I would like to land this this weekend, so I have time to watch and fix in case
something goes wrong in the main waterfall.

Thank you!

[Avi (use Gerrit), 2014-08-08 22:30:55]

stamp lgtm

https://codereview.chromium.org/423613003/diff/350001/components/components_t...
File components/components_tests.gyp (right):

https://codereview.chromium.org/423613003/diff/350001/components/components_t...
components/components_tests.gyp:857:
'<(SHARED_INTERMEDIATE_DIR)/content/app/strings/content_strings_en-US.rc',
sort?

[tfarina, 2014-08-09 16:29:43]

TBRing Darin for the rest.

Pushing to CQ!

https://codereview.chromium.org/423613003/diff/350001/components/components_t...
File components/components_tests.gyp (right):

https://codereview.chromium.org/423613003/diff/350001/components/components_t...
components/components_tests.gyp:857:
'<(SHARED_INTERMEDIATE_DIR)/content/app/strings/content_strings_en-US.rc',
On 2014/08/08 22:30:55, Avi wrote:
> sort?

Done.

[tfarina, 2014-08-09 16:30:07]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-08-09 16:31:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tfarina@chromium.org/423613003/390001

[commit-bot: I haz the power, 2014-08-09 19:24:18]

Change committed as 288589