Reland http://codereview.chromium.org/9121050/ with fix for aura_win

Reland http://codereview.chromium.org/9121050/ with fix for aura_win

Add accessible names to the launcher buttons


BUG=104192
TEST=None

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

[Jói, 2012-02-03 09:05:57]

I don't see anything obviously wrong, except one thing below that wouldn't be
causing problems building with the unit tests.

Suggestions for tracking down:

a) Open the .exe or .dll in Visual Studio (as in File / Open...) which will give
you a view of its sections, you can look into the resources and see if the
appropriate ID is there (you'll need to get its numeric value from the header
file).  Is the resource there?

b) See if all the output files you expect get output to the places you expect
them.

c) See if Visual Studio complains of any missing input files.  If you find the
input files in the files view (tree view on the left hand side of Visual
Studio), does double-clicking them work and open the right files?

d) Does a fresh build work?  If not, do things work if you build a couple of
times without cleaning in between?  If so, there is a dependency ordering
problem somewhere.

If you'd like further help, please send more details on the exact problems you
are seeing.

Another person who is very familiar with the way grit is integrated into Chrome
is Tony Chang.  He might be able to offer some insight.

Cheers,
Jói

https://chromiumcodereview.appspot.com/9307037/diff/5002/ash/ash.gyp
File ash/ash.gyp (right):

https://chromiumcodereview.appspot.com/9307037/diff/5002/ash/ash.gyp#newcode313
ash/ash.gyp:313: '<(SHARED_INTERMEDIATE_DIR)/ui/gfx/gfx_resources.rc',
presumably, you would also need ash_strings.rc here

[Zachary Kuznia, 2012-02-06 09:36:43]

Hello sky,

Could you review this at:
https://chromiumcodereview.appspot.com/9307037

I've removed the dependency on the strings by aura_shell_tests.  Other that
that, it's just the combination of the original CL and the second CL to fix the
linux_aura build.

Thanks,
-Zach

[sky, 2012-02-06 16:32:29]

Tony should review this as he knows more about strings than I. I'm rubber
stamping, but wait for Tony before landing: LGTM

[tony, 2012-02-06 18:37:20]

https://chromiumcodereview.appspot.com/9307037/diff/5002/ash/test/test_suite.cc
File ash/test/test_suite.cc (right):

https://chromiumcodereview.appspot.com/9307037/diff/5002/ash/test/test_suite....
ash/test/test_suite.cc:38: PathService::Override(ui::DIR_LOCALES, pak_dir);
As I mentioned in https://chromiumcodereview.appspot.com/9316029, rather than
generating a .rc file that is only used by windows, you can generate the pak
file on all platforms and use ResourceBundle::InitSharedInstaceWithPakFile() to
load aura_shell_unittests_strings.  This will make the build and code the same
on all platforms.  pak files work on Windows too.

[Zachary Kuznia, 2012-02-06 23:13:57]

I've removed the dependency in aura_shell_tests entirely.

[tony, 2012-02-06 23:16:48]

LGTM

https://chromiumcodereview.appspot.com/9307037/diff/13001/ash/launcher/launch...
File ash/launcher/launcher_view.cc (right):

https://chromiumcodereview.appspot.com/9307037/diff/13001/ash/launcher/launch...
ash/launcher/launcher_view.cc:669: NOTREACHED();
Nit: If you remove default:, you'll get a compiler error if someone adds a type
and forgets to update this switch.

[Zachary Kuznia, 2012-02-07 08:32:21]

Tony, could you have a quick look at the changes to src/chrome/?

https://chromiumcodereview.appspot.com/9307037/diff/13001/ash/launcher/launch...
File ash/launcher/launcher_view.cc (right):

https://chromiumcodereview.appspot.com/9307037/diff/13001/ash/launcher/launch...
ash/launcher/launcher_view.cc:669: NOTREACHED();
On 2012/02/06 23:16:48, tony wrote:
> Nit: If you remove default:, you'll get a compiler error if someone adds a
type
> and forgets to update this switch.

Done.

[tony, 2012-02-07 18:15:59]

https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/chrome_brows...
File chrome/chrome_browser.gypi (right):

https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/chrome_brows...
chrome/chrome_browser.gypi:33: '../ash/ash.gyp:ash_strings',
Is this needed?  Does a browser .cc file actually include grit/ash_strings.h?

https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/chrome_resou...
File chrome/chrome_resources.gyp (right):

https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/chrome_resou...
chrome/chrome_resources.gyp:273: '<(DEPTH)/ash/ash.gyp:ash_strings',
Should it be protected by use_ash==1?  why not do this change now?

https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/tools/build/...
File chrome/tools/build/repack_locales.py (right):

https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/tools/build/...
chrome/tools/build/repack_locales.py:75: #e.g.
'<(SHARED_INTERMEDIATE_DIR)/ash_strings_da.pak',
This comment is incorrect.  It's
'<(SHARED_INTERMEDIATE_DIR)/ash_strings/ash_strings_da.pak'.

[Zachary Kuznia, 2012-02-07 22:13:54]

On Wed, Feb 8, 2012 at 3:16 AM, <tony@chromium.org> wrote:

>
> https://chromiumcodereview.**appspot.com/9307037/diff/**
>
18001/chrome/chrome_browser.**gypi<https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/chrome_browser.gypi>
> File chrome/chrome_browser.gypi (right):
>
> https://chromiumcodereview.**appspot.com/9307037/diff/**
>
18001/chrome/chrome_browser.**gypi#newcode33<https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/chrome_browser.gypi#newcode33>
> chrome/chrome_browser.gypi:33: '../ash/ash.gyp:ash_strings',
> Is this needed?  Does a browser .cc file actually include
> grit/ash_strings.h?
>

I'll remove this dependency.


>
> https://chromiumcodereview.**appspot.com/9307037/diff/**
>
18001/chrome/chrome_resources.**gyp<https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/chrome_resources.gyp>
> File chrome/chrome_resources.gyp (right):
>
> https://chromiumcodereview.**appspot.com/9307037/diff/**
>
18001/chrome/chrome_resources.**gyp#newcode273<https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/chrome_resources.gyp#newcode273>
> chrome/chrome_resources.gyp:**273: '<(DEPTH)/ash/ash.gyp:ash_**strings',
> Should it be protected by use_ash==1?  why not do this change now?
>

As discussed, I'll add a todo for this in the grd file.


>
> https://chromiumcodereview.**appspot.com/9307037/diff/**
>
18001/chrome/tools/build/**repack_locales.py<https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/tools/build/repack_locales.py>
> File chrome/tools/build/repack_**locales.py (right):
>
> https://chromiumcodereview.**appspot.com/9307037/diff/**
>
18001/chrome/tools/build/**repack_locales.py#newcode75<https://chromiumcodereview.appspot.com/9307037/diff/18001/chrome/tools/build/repack_locales.py#newcode75>
> chrome/tools/build/repack_**locales.py:75: #e.g.
> '<(SHARED_INTERMEDIATE_DIR)/**ash_strings_da.pak',
> This comment is incorrect.  It's
> '<(SHARED_INTERMEDIATE_DIR)/**ash_strings/ash_strings_da.**pak'.
>

I'll fix this.


>
>
https://chromiumcodereview.**appspot.com/9307037/<https://chromiumcodereview....
>

[commit-bot: I haz the power, 2012-02-08 03:36:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zork@chromium.org/9307037/31001

[commit-bot: I haz the power, 2012-02-08 06:27:37]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is win_rel, revision is 120941, job name
was 9307037-31001 (retry) (previous was lost) (previous was lost) (previous was
lost).

[commit-bot: I haz the power, 2012-02-08 06:49:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zork@chromium.org/9307037/31001