Cache ShellImpl by resolved URL, not initial URL

Cache ShellImpl by resolved URL, not initial URL

This prevents us from loading applications twice when URLs are mapped.

BUG=427974
R=davemoore@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/8c71ea8f07989d102ea575f4d75947392900c414

Committed: https://chromium.googlesource.com/external/mojo/+/cd7b911cdf4ff5ada2d0fdc0c026a508d3cdfe5d

[Aaron Boodman, 2014-10-31 05:20:40]

aa@chromium.org changed reviewers:
+ davemoore@chromium.org

[Aaron Boodman, 2014-10-31 05:20:47]

Patchset #1 (id:1) has been deleted

[Aaron Boodman, 2014-10-31 05:20:55]

Patchset #1 (id:20001) has been deleted

[Aaron Boodman, 2014-10-31 05:25:53]

Patchset #1 (id:40001) has been deleted

[Aaron Boodman, 2014-10-31 05:26:17]

https://codereview.chromium.org/696563002/diff/60001/mojo/application_manager...
File mojo/application_manager/application_manager.h (left):

https://codereview.chromium.org/696563002/diff/60001/mojo/application_manager...
mojo/application_manager/application_manager.h:61: // Returns a shared instance,
creating it if necessary.
Neither of these were called from anywhere.

[qsr, 2014-10-31 12:22:15]

qsr@chromium.org changed reviewers:
+ qsr@chromium.org

[qsr, 2014-10-31 12:22:16]

https://codereview.chromium.org/696563002/diff/60001/mojo/shell/context.cc
File mojo/shell/context.cc (right):

https://codereview.chromium.org/696563002/diff/60001/mojo/shell/context.cc#ne...
mojo/shell/context.cc:162: Context::Context() : application_manager_(NULL) {
s/NULL/this/ ?

[DaveMoore, 2014-10-31 16:15:58]

https://codereview.chromium.org/696563002/diff/60001/mojo/application_manager...
File mojo/application_manager/application_manager.cc (right):

https://codereview.chromium.org/696563002/diff/60001/mojo/application_manager...
mojo/application_manager/application_manager.cc:101:
ShellImpl(ApplicationManager* manager, const GURL& requested_url,
Nit: could you put these on their own lines?

[Aaron Boodman, 2014-10-31 18:14:02]

done

I also rebased, sorry

https://codereview.chromium.org/696563002/diff/60001/mojo/application_manager...
File mojo/application_manager/application_manager.cc (right):

https://codereview.chromium.org/696563002/diff/60001/mojo/application_manager...
mojo/application_manager/application_manager.cc:101:
ShellImpl(ApplicationManager* manager, const GURL& requested_url,
On 2014/10/31 16:15:58, DaveMoore wrote:
> Nit: could you put these on their own lines?

Done.

https://codereview.chromium.org/696563002/diff/60001/mojo/shell/context.cc
File mojo/shell/context.cc (right):

https://codereview.chromium.org/696563002/diff/60001/mojo/shell/context.cc#ne...
mojo/shell/context.cc:162: Context::Context() : application_manager_(NULL) {
On 2014/10/31 12:22:15, qsr wrote:
> s/NULL/this/ ?

Thanks, I went back and forth on how to do this and forgot to fix this last
instance.

[Aaron Boodman, 2014-10-31 18:30:17]

Also, please excuse the red bot noise. I forgot we don't have bots.

[DaveMoore, 2014-10-31 18:37:39]

lgtm

[Aaron Boodman, 2014-10-31 18:47:47]

Committed patchset #2 (id:80001) manually as
8c71ea8f07989d102ea575f4d75947392900c414 (presubmit successful).

[Aaron Boodman, 2014-11-07 05:44:58]

Committed patchset #3 (id:100001) manually as
cd7b911cdf4ff5ada2d0fdc0c026a508d3cdfe5d (presubmit successful).