Introduce extensions_strings.grd

Introduce extensions_strings.grd

Extract extensions-module strings into their own .grd and .gyp file.
Follows the conventions of components_strings.gyp and ui_strings.gyp.
Starts by moving sockets API strings, more will follow.

BUG=335157
TEST=compiles, unit_tests *Extension*
TBR=mmoss@chromium.org for chrome/tools/build/repack_locales.py
TBR=thomasvl@chromium.org for chrome/tools/check_grd_for_unused_strings.py

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

[James Cook, 2014-04-15 23:46:39]

tfarina, PTAL, especially at how I set up the gyp and grd files.  It looks like
you did some similar things recently for components and bluetooth strings?

yoz, PTAL at the DEPS and as an extensions OWNER.

https://codereview.chromium.org/239543005/diff/1/extensions/extensions.gyp
File extensions/extensions.gyp (right):

https://codereview.chromium.org/239543005/diff/1/extensions/extensions.gyp#ne...
extensions/extensions.gyp:35: 'common/api/api.gyp:extensions_api',
I sorted this so everything is alphabetical.

https://codereview.chromium.org/239543005/diff/1/extensions/extensions.gyp#ne...
extensions/extensions.gyp:480: '../testing/gmock.gyp:gmock',
I missed this before -- a couple tests below use gmock.

https://codereview.chromium.org/239543005/diff/1/extensions/strings/extension...
File extensions/strings/extensions_strings_am.xtb (right):

https://codereview.chromium.org/239543005/diff/1/extensions/strings/extension...
extensions/strings/extensions_strings_am.xtb:1: <?xml version="1.0" ?>
I am unsure if I can just provide empty stubs for all these files or if I need
to find every translation in the chrome .xtb files and copy/paste them over. 
tfarina, do you know?

I didn't do that because I was worried about screwing up the Unicode chars, but
if you think it's necessary I can figure it out.

https://codereview.chromium.org/239543005/diff/1/tools/gritsettings/resource_ids
File tools/gritsettings/resource_ids (right):

https://codereview.chromium.org/239543005/diff/1/tools/gritsettings/resource_...
tools/gritsettings/resource_ids:170: "messages": [27000],
I doubt we will have 500 strings. I mostly wanted to reserve some space in the
26500-27000 range so I can stick extensions images and JSON files here later.

[James Cook, 2014-04-15 23:49:38]

+derat just as FYI

[Yoyo Zhou, 2014-04-16 00:07:02]

LGTM as a first pass

https://codereview.chromium.org/239543005/diff/1/extensions/extensions_string...
File extensions/extensions_strings.gyp (right):

https://codereview.chromium.org/239543005/diff/1/extensions/extensions_string...
extensions/extensions_strings.gyp:24: },
Shouldn't this be a hard dependency?

https://codereview.chromium.org/239543005/diff/1/tools/gritsettings/resource_ids
File tools/gritsettings/resource_ids (right):

https://codereview.chromium.org/239543005/diff/1/tools/gritsettings/resource_...
tools/gritsettings/resource_ids:170: "messages": [27000],
On 2014/04/15 23:46:39, James Cook wrote:
> I doubt we will have 500 strings. I mostly wanted to reserve some space in the
> 26500-27000 range so I can stick extensions images and JSON files here later.

I don't really know how to choose numbers here...

[tfarina, 2014-04-16 02:30:56]

James, I'm adding Tony to answer your question below.

Also I'm trying to use the term "messages" rather than "strings" throughout, but
haven't had time to fix the existing places. I won't go into detail, but I
wanted to use "messages" instead of "strings", because that is the tag name used
inside the xml files.

Also, I would love if the xtb (xml translation bundle) files could go into
extensions/resources/translations/ directories. I'm also trying to make that
layout, the standard throught, but haven't started yet.

I have a bug filed for this though and I plan to do for ui, components and ash.
If you could start here, that would be awesome!

https://codereview.chromium.org/239543005/diff/1/extensions/strings/extension...
File extensions/strings/extensions_strings_am.xtb (right):

https://codereview.chromium.org/239543005/diff/1/extensions/strings/extension...
extensions/strings/extensions_strings_am.xtb:1: <?xml version="1.0" ?>
On 2014/04/15 23:46:39, James Cook wrote:
> I am unsure if I can just provide empty stubs for all these files or if I need
> to find every translation in the chrome .xtb files and copy/paste them over. 
> tfarina, do you know?
> 
I don't know for sure. What I know is that this goes into a tool called
"console", which I think it is a internal Java tool used by the translators.

I think you can leave them empty, and they will be translated later.

We should check with Tony to be sure though.

> I didn't do that because I was worried about screwing up the Unicode chars,
but
> if you think it's necessary I can figure it out.

[Daniel Erat, 2014-04-16 02:36:47]

By "use the term 'messages' rather than 'strings'", are you referring to
filenames? I thought that we generally use "messages" to refer to IPC stuff (see
e.g. extensions/common/extension_messages.h) and "strings" to refer to
translatable resources.

[tfarina, 2014-04-16 02:43:30]

Daniel, yes -> extensions_strings.grd -> extensions_messages.grd

May be you have a point though. Haven't thought about the IPC case and how that
might confuse some people.

[Daniel Erat, 2014-04-16 02:46:03]

On 2014/04/16 02:43:30, tfarina wrote:
> Daniel, yes -> extensions_strings.grd -> extensions_messages.grd
> 
> May be you have a point though. Haven't thought about the IPC case and how
that
> might confuse some people.

Yes, please don't do this. Running "git ls-files | grep _messages.h" and "git
ls-files | grep _strings.h" is a compelling argument against it.

[James Cook, 2014-04-16 15:36:55]

On 2014/04/16 02:46:03, Daniel Erat wrote:
> On 2014/04/16 02:43:30, tfarina wrote:
> > Daniel, yes -> extensions_strings.grd -> extensions_messages.grd
> > 
> > May be you have a point though. Haven't thought about the IPC case and how
> that
> > might confuse some people.
> 
> Yes, please don't do this. Running "git ls-files | grep _messages.h" and "git
> ls-files | grep _strings.h" is a compelling argument against it.

Yes, I would prefer to follow the pattern in components (components_strings and
xtb in //components/strings) which is also used in ash and devices/bluetooth.  I
agree with Dan's suggestion about IPC.

[tfarina, 2014-04-16 16:04:14]

OK. I'm fine following existing scheme.

[tfarina, 2014-04-16 16:08:11]

https://codereview.chromium.org/239543005/diff/20001/extensions/extensions_st...
File extensions/extensions_strings.gyp (right):

https://codereview.chromium.org/239543005/diff/20001/extensions/extensions_st...
extensions/extensions_strings.gyp:22:
'<(SHARED_INTERMEDIATE_DIR)/extensions_strings',
did you deliberated choose not to use the path (extensions/strings)? if yes,
what was the reason? I'm just curious to know if you had any reason, because it
seems to work for components.

https://codereview.chromium.org/239543005/diff/20001/extensions/extensions_st...
extensions/extensions_strings.gyp:25: # This target generates
extensions_strings.h so it must build before
My English is not all that good, but is it incorrect to write
s/must build/must be built?

[James Cook, 2014-04-16 16:22:34]

https://codereview.chromium.org/239543005/diff/1/extensions/extensions_string...
File extensions/extensions_strings.gyp (right):

https://codereview.chromium.org/239543005/diff/1/extensions/extensions_string...
extensions/extensions_strings.gyp:24: },
On 2014/04/16 00:07:03, Yoyo Zhou wrote:
> Shouldn't this be a hard dependency?

Yes. Done.

https://codereview.chromium.org/239543005/diff/20001/extensions/extensions_st...
File extensions/extensions_strings.gyp (right):

https://codereview.chromium.org/239543005/diff/20001/extensions/extensions_st...
extensions/extensions_strings.gyp:22:
'<(SHARED_INTERMEDIATE_DIR)/extensions_strings',
On 2014/04/16 16:08:12, tfarina wrote:
> did you deliberated choose not to use the path (extensions/strings)? if yes,
> what was the reason? I'm just curious to know if you had any reason, because
it
> seems to work for components.

No good reason. Done.

https://codereview.chromium.org/239543005/diff/20001/extensions/extensions_st...
extensions/extensions_strings.gyp:25: # This target generates
extensions_strings.h so it must build before
On 2014/04/16 16:08:12, tfarina wrote:
> My English is not all that good, but is it incorrect to write
> s/must build/must be built?

Done. Both are correct but your version sounds better.

[tfarina, 2014-04-16 18:09:07]

Did everything run fine? Aren't we missing any extra setup with ResourceBundle?

Please, also run the trybots.

I think this lgtm assuming you have tested this locally and everything looks
fine as before.

Thanks James!

https://codereview.chromium.org/239543005/diff/60001/extensions/browser/DEPS
File extensions/browser/DEPS (right):

https://codereview.chromium.org/239543005/diff/60001/extensions/browser/DEPS#...
extensions/browser/DEPS:5: "+grit/extensions_strings.h",
do you need this just yet? I don't see extensions/browser/ including
grit/extensions_strings.h, just extensions/common/

[tony, 2014-04-16 18:38:11]

Stub XTB files are fine.  The translations will be broken on trunk, but before
the next release, they should get updated properly by the TPM.

[James Cook, 2014-04-16 21:08:35]

yoz, could you take another quick look?  I moved all the IDS strings referenced
in src/extensions which allowed me to eliminate the dependency on chrome_strings
and tighten the DEPS files.

https://codereview.chromium.org/239543005/diff/60001/extensions/browser/DEPS
File extensions/browser/DEPS (right):

https://codereview.chromium.org/239543005/diff/60001/extensions/browser/DEPS#...
extensions/browser/DEPS:5: "+grit/extensions_strings.h",
On 2014/04/16 18:09:08, tfarina wrote:
> do you need this just yet? I don't see extensions/browser/ including
> grit/extensions_strings.h, just extensions/common/

I found another couple strings in extensions/browser, so we'll need this.

[Yoyo Zhou, 2014-04-16 21:55:53]

still LGTM

[James Cook, 2014-04-17 02:54:48]

The CQ bit was checked by jamescook@chromium.org

[commit-bot: I haz the power, 2014-04-17 02:55:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamescook@chromium.org/239543005/110001

[commit-bot: I haz the power, 2014-04-17 05:22:01]

Change committed as 264430

[tfarina, 2014-04-17 16:12:22]

Idea for a follow up CL.

https://codereview.chromium.org/239543005/diff/110001/extensions/common/file_...
File extensions/common/file_util_unittest.cc (right):

https://codereview.chromium.org/239543005/diff/110001/extensions/common/file_...
extensions/common/file_util_unittest.cc:14: #include
"chrome/common/chrome_paths.h"
We should remove this. We are using it for chrome::DIR_TEST_DATA. I think we
need to create a extensions_paths.*:RegisterPathProvider and
extensions/test/data directory to fix this.

[James Cook, 2014-04-17 17:31:13]

On 2014/04/17 16:12:22, tfarina wrote:
> Idea for a follow up CL.
> 
>
https://codereview.chromium.org/239543005/diff/110001/extensions/common/file_...
> File extensions/common/file_util_unittest.cc (right):
> 
>
https://codereview.chromium.org/239543005/diff/110001/extensions/common/file_...
> extensions/common/file_util_unittest.cc:14: #include
> "chrome/common/chrome_paths.h"
> We should remove this. We are using it for chrome::DIR_TEST_DATA. I think we
> need to create a extensions_paths.*:RegisterPathProvider and
> extensions/test/data directory to fix this.

That seems OK but keep in mind that Chrome unit_tests and extensions_unittests
share a lot of the same data files. Moving one piece of test data to
//extensions/test/data will involve updating tests that stay in chrome.