chrome/mash: Load resources before running the mus app.

chrome/mash: Load resources before running the mus app.

When running 'chrome --mash' on device, all the resource files necessary
for ash, mus etc. apps are already in the resource files used for regular
chrome. So instead of deploying separate resource files for these apps,
just load chrome's resource files, before initializing the app.

BUG=628715, 633656
Committed: https://crrev.com/6a570def6dbb73675bdbee3766e636f0f2218439
Cr-Commit-Position: refs/heads/master@{#422694}

[sadrul, 2016-10-03 21:09:34]

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

[commit-bot: I haz the power, 2016-10-03 21:10:34]

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, 2016-10-03 21:39:15]

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

[commit-bot: I haz the power, 2016-10-03 21:39:16]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[sadrul, 2016-10-03 21:43:43]

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

[sadrul, 2016-10-03 21:44:03]

sadrul@chromium.org changed reviewers:
+ msw@chromium.org, sky@chromium.org

[commit-bot: I haz the power, 2016-10-03 21:44:45]

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

[msw, 2016-10-03 22:07:39]

https://codereview.chromium.org/2387233002/diff/20001/chrome/app/mash/mash_ru...
File chrome/app/mash/mash_runner.cc (right):

https://codereview.chromium.org/2387233002/diff/20001/chrome/app/mash/mash_ru...
chrome/app/mash/mash_runner.cc:55:
ui::ResourceBundle::InitSharedInstanceWithLocale(
nit: add a comment that this loads Chrome's resource paks.

https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init.cc
File ui/views/mus/aura_init.cc (right):

https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init....
ui/views/mus/aura_init.cc:83: void
AuraInit::InitializeResources(shell::Connector* connector) {
q: Will this fail on devices without the specified resource files?
(should we skip this on devices? log if the files aren't found, etc?)

[sky, 2016-10-03 22:17:17]

LGTM

https://codereview.chromium.org/2387233002/diff/20001/chrome/app/mash/mash_ru...
File chrome/app/mash/mash_runner.cc (right):

https://codereview.chromium.org/2387233002/diff/20001/chrome/app/mash/mash_ru...
chrome/app/mash/mash_runner.cc:56: locale, NULL,
ui::ResourceBundle::LOAD_COMMON_RESOURCES);
nullptr

[commit-bot: I haz the power, 2016-10-03 22:28:22]

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

[commit-bot: I haz the power, 2016-10-03 22:28:23]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-10-04 00:42:42]

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

[sadrul, 2016-10-04 00:42:48]

https://codereview.chromium.org/2387233002/diff/20001/chrome/app/mash/mash_ru...
File chrome/app/mash/mash_runner.cc (right):

https://codereview.chromium.org/2387233002/diff/20001/chrome/app/mash/mash_ru...
chrome/app/mash/mash_runner.cc:55:
ui::ResourceBundle::InitSharedInstanceWithLocale(
On 2016/10/03 22:07:39, msw wrote:
> nit: add a comment that this loads Chrome's resource paks.

Done.

https://codereview.chromium.org/2387233002/diff/20001/chrome/app/mash/mash_ru...
chrome/app/mash/mash_runner.cc:56: locale, NULL,
ui::ResourceBundle::LOAD_COMMON_RESOURCES);
On 2016/10/03 22:17:17, sky wrote:
> nullptr

Done.

https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init.cc
File ui/views/mus/aura_init.cc (right):

https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init....
ui/views/mus/aura_init.cc:83: void
AuraInit::InitializeResources(shell::Connector* connector) {
On 2016/10/03 22:07:39, msw wrote:
> q: Will this fail on devices without the specified resource files?
> (should we skip this on devices? log if the files aren't found, etc?)

On device, we can only run with chrome --mash (since we no longer deploy
mojo_runner). And when running from chrome, we will already have loaded the
chrome resources when we get here. That means this function will not do
anything. (early return after the RB::HasSharedInstance() check)

[commit-bot: I haz the power, 2016-10-04 00:43:21]

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

[msw, 2016-10-04 00:47:59]

lgtm; it'd be nice if we could express this data dependency, but I'm not sure
it's feasible. wdyt?

https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init.cc
File ui/views/mus/aura_init.cc (right):

https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init....
ui/views/mus/aura_init.cc:83: void
AuraInit::InitializeResources(shell::Connector* connector) {
On 2016/10/04 00:42:48, sadrul wrote:
> On 2016/10/03 22:07:39, msw wrote:
> > q: Will this fail on devices without the specified resource files?
> > (should we skip this on devices? log if the files aren't found, etc?)
> 
> On device, we can only run with chrome --mash (since we no longer deploy
> mojo_runner). And when running from chrome, we will already have loaded the
> chrome resources when we get here. That means this function will not do
> anything. (early return after the RB::HasSharedInstance() check)

Ah, I didn't catch the early return; maybe comment there? thanks!

[sadrul, 2016-10-04 01:04:05]

On 2016/10/04 00:47:59, msw wrote:
> lgtm; it'd be nice if we could express this data dependency, but I'm not sure
> it's feasible. wdyt?

I think the tricky part here is that we want the same build configuration to
work for both mojo_runner on dev-workstation, and for 'chrome --mash' on device.
So as far as data dependency goes, we still need to make sure the app-specific
resource-paks (e.g. ash_mus_resources etc.) get created and installed in the
right output location. It's just that we don't always need these resources.

> 
>
https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init.cc
> File ui/views/mus/aura_init.cc (right):
> 
>
https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init....
> ui/views/mus/aura_init.cc:83: void
> AuraInit::InitializeResources(shell::Connector* connector) {
> On 2016/10/04 00:42:48, sadrul wrote:
> > On 2016/10/03 22:07:39, msw wrote:
> > > q: Will this fail on devices without the specified resource files?
> > > (should we skip this on devices? log if the files aren't found, etc?)
> > 
> > On device, we can only run with chrome --mash (since we no longer deploy
> > mojo_runner). And when running from chrome, we will already have loaded the
> > chrome resources when we get here. That means this function will not do
> > anything. (early return after the RB::HasSharedInstance() check)
> 
> Ah, I didn't catch the early return; maybe comment there? thanks!

[commit-bot: I haz the power, 2016-10-04 01:30:37]

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

[commit-bot: I haz the power, 2016-10-04 01:30:38]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-10-04 01:56:18]

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

[msw, 2016-10-04 01:56:45]

On 2016/10/04 01:04:05, sadrul wrote:
> On 2016/10/04 00:47:59, msw wrote:
> > lgtm; it'd be nice if we could express this data dependency, but I'm not
sure
> > it's feasible. wdyt?
> 
> I think the tricky part here is that we want the same build configuration to
> work for both mojo_runner on dev-workstation, and for 'chrome --mash' on
device.
> So as far as data dependency goes, we still need to make sure the app-specific
> resource-paks (e.g. ash_mus_resources etc.) get created and installed in the
> right output location. It's just that we don't always need these resources.

I actually meant the dependency mojo:ash now has on chrome resources.
It may not be worth addressing; afaik, it's always had this underspecified dep.

[sadrul, 2016-10-04 01:56:47]

https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init.cc
File ui/views/mus/aura_init.cc (right):

https://codereview.chromium.org/2387233002/diff/20001/ui/views/mus/aura_init....
ui/views/mus/aura_init.cc:83: void
AuraInit::InitializeResources(shell::Connector* connector) {
On 2016/10/04 00:47:59, msw wrote:
> On 2016/10/04 00:42:48, sadrul wrote:
> > On 2016/10/03 22:07:39, msw wrote:
> > > q: Will this fail on devices without the specified resource files?
> > > (should we skip this on devices? log if the files aren't found, etc?)
> > 
> > On device, we can only run with chrome --mash (since we no longer deploy
> > mojo_runner). And when running from chrome, we will already have loaded the
> > chrome resources when we get here. That means this function will not do
> > anything. (early return after the RB::HasSharedInstance() check)
> 
> Ah, I didn't catch the early return; maybe comment there? thanks!

Added a comment.

[commit-bot: I haz the power, 2016-10-04 01:56:54]

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, 2016-10-04 02:34:18]

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

[commit-bot: I haz the power, 2016-10-04 02:34:19]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-10-04 03:28:29]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-10-04 03:28:30]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2387233002/#ps60001
(title: ".")

[commit-bot: I haz the power, 2016-10-04 03:29:52]

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, 2016-10-04 03:45:15]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-04 03:47:37]

Description was changed from

==========
chrome/mash: Load resources before running the mus app.

When running 'chrome --mash' on device, all the resource files necessary
for ash, mus etc. apps are already in the resource files used for regular
chrome. So instead of deploying separate resource files for these apps,
just load chrome's resource files, before initializing the app.

BUG=628715, 633656
==========

to

==========
chrome/mash: Load resources before running the mus app.

When running 'chrome --mash' on device, all the resource files necessary
for ash, mus etc. apps are already in the resource files used for regular
chrome. So instead of deploying separate resource files for these apps,
just load chrome's resource files, before initializing the app.

BUG=628715, 633656

Committed: https://crrev.com/6a570def6dbb73675bdbee3766e636f0f2218439
Cr-Commit-Position: refs/heads/master@{#422694}
==========

[commit-bot: I haz the power, 2016-10-04 03:47:38]

Patchset 4 (id:??) landed as
https://crrev.com/6a570def6dbb73675bdbee3766e636f0f2218439
Cr-Commit-Position: refs/heads/master@{#422694}