[App Shell] Introduce app_shell.pak

[App Shell] Introduce app_shell.pak

This patch is to introduce an app_shell.pak to include dev-tools
needed resources, instead of entire content_shell.pak.

Next step: Also include chrome-specific needed resources and removing
chrome resources.pak.

BUG=344974
TEST=launch app_shell with '--remote-debugging-port', and open the
debug page.

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

[Haojian Wu, 2014-02-20 15:46:42]

James@, please review it, Thanks.

https://codereview.chromium.org/173883002/diff/1/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/173883002/diff/1/apps/apps.gypi#newcode106
apps/apps.gypi:106:
'../content/content_shell_and_tests.gyp:generate_content_shell_resources',
Here we only include the minimal dev-tools related resources in content layer,
and repack it to a new app_shell.pak.

There is an optional solution: we can just copy the related resources' files to
app/shell/resources/ and build app_shell's own pak file. But this way may make
depulicated files, also we may not reuse the classes in content_shell directly
since they use content_shell resources. 

Which way do you prefer, James?

[tfarina, 2014-02-20 18:44:00]

https://codereview.chromium.org/173883002/diff/1/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/173883002/diff/1/apps/apps.gypi#newcode103
apps/apps.gypi:103: 'target_name': 'app_shell_resources',
no, not resources, it is pak. It would be resources if you were generating it
from the grd file.

so name this app_shell_pak.

[tfarina, 2014-02-20 18:46:14]

https://codereview.chromium.org/173883002/diff/1/apps/shell/app/shell_main_de...
File apps/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/173883002/diff/1/apps/shell/app/shell_main_de...
apps/shell/app/shell_main_delegate.cc:100: base::FilePath pak_file, pak_dir;
Please, clean this up.

Like this:

base::FilePath pak_dir;
PathService::Get(base::DIR_MODULE, &pak_dir);
base::FilePath pak_file = pak_dir.AppendASCII("app_shell.pak");
...

you could even inline pak_file directly in AddPackFromPath().

[James Cook, 2014-02-20 19:30:00]

yoz, do you have thoughts about how we create a PAK file for app_shell?

Eventually we'll need some way to get our own strings / art / extension manifest
data into a PAK.  However, it seems like some of that should live at the
src/extensions level and some at the src/apps level.

This seems like a step in the right direction to me -- we probably want a single
.PAK for app_shell, so we'll need to repack the bits of content_shell and other
resources we need.

[Yoyo Zhou, 2014-02-20 21:27:24]

On 2014/02/20 19:30:00, James Cook wrote:
> yoz, do you have thoughts about how we create a PAK file for app_shell?
> 
> Eventually we'll need some way to get our own strings / art / extension
manifest
> data into a PAK.  However, it seems like some of that should live at the
> src/extensions level and some at the src/apps level.
> 
> This seems like a step in the right direction to me -- we probably want a
single
> .PAK for app_shell, so we'll need to repack the bits of content_shell and
other
> resources we need.

Seems like a fine start to me.

[James Cook, 2014-02-20 21:31:43]

I think this approach is fine -- duplicated files are bad.

LGTM but please address tfarina's comments before committing.

[Haojian Wu, 2014-02-21 05:50:01]

https://codereview.chromium.org/173883002/diff/1/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/173883002/diff/1/apps/apps.gypi#newcode103
apps/apps.gypi:103: 'target_name': 'app_shell_resources',
On 2014/02/20 18:44:01, tfarina wrote:
> no, not resources, it is pak. It would be resources if you were generating it
> from the grd file.
> 
> so name this app_shell_pak.

Done.

https://codereview.chromium.org/173883002/diff/1/apps/shell/app/shell_main_de...
File apps/shell/app/shell_main_delegate.cc (right):

https://codereview.chromium.org/173883002/diff/1/apps/shell/app/shell_main_de...
apps/shell/app/shell_main_delegate.cc:100: base::FilePath pak_file, pak_dir;
On 2014/02/20 18:46:15, tfarina wrote:
> Please, clean this up.
> 
> Like this:
> 
> base::FilePath pak_dir;
> PathService::Get(base::DIR_MODULE, &pak_dir);
> base::FilePath pak_file = pak_dir.AppendASCII("app_shell.pak");
> ...
> 
> you could even inline pak_file directly in AddPackFromPath().

Done.

[Haojian Wu, 2014-02-21 07:22:04]

The CQ bit was checked by hokein.wu@gmail.com

[commit-bot: I haz the power, 2014-02-21 07:22:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/Hokein.Wu@gmail.com/173883002/110001

[commit-bot: I haz the power, 2014-02-21 14:55:35]

Change committed as 252553