Changes html_viewer to get services from appropriate application

Changes html_viewer to get services from appropriate application

There are actually two changes here:
. Convert HTMLViewerApplication to use an ApplicationImpl rather than
  be an Application.
. Make HTMLDocument use services from the application it was created
  for rather than HTMLViewer's application.

BUG=none
TEST=none
R=jam@chromium.org

Committed: https://crrev.com/15df477605e2afc0abf2b05074c8ecbd8ac28c29
Cr-Commit-Position: refs/heads/master@{#333050}

[jam, 2015-06-04 22:10:00]

https://codereview.chromium.org/1165013005/diff/20001/components/html_viewer/...
File components/html_viewer/html_document.h (right):

https://codereview.chromium.org/1165013005/diff/20001/components/html_viewer/...
components/html_viewer/html_document.h:62: mojo::ApplicationConnection*
connection,
nit: update comment.

https://codereview.chromium.org/1165013005/diff/20001/components/html_viewer/...
File components/html_viewer/html_viewer.cc (right):

https://codereview.chromium.org/1165013005/diff/20001/components/html_viewer/...
components/html_viewer/html_viewer.cc:67: void DoQuit() { delete this; }
why isn't this app_->shell()->QuitApplication() as before so that it goes
through the standard app shutdown?

[sky, 2015-06-04 23:16:46]

https://codereview.chromium.org/1165013005/diff/20001/components/html_viewer/...
File components/html_viewer/html_document.h (right):

https://codereview.chromium.org/1165013005/diff/20001/components/html_viewer/...
components/html_viewer/html_document.h:62: mojo::ApplicationConnection*
connection,
On 2015/06/04 22:10:00, jam wrote:
> nit: update comment. 

Done.

https://codereview.chromium.org/1165013005/diff/20001/components/html_viewer/...
File components/html_viewer/html_viewer.cc (right):

https://codereview.chromium.org/1165013005/diff/20001/components/html_viewer/...
components/html_viewer/html_viewer.cc:67: void DoQuit() { delete this; }
On 2015/06/04 22:10:00, jam wrote:
> why isn't this app_->shell()->QuitApplication() as before so that it goes
> through the standard app shutdown?

As discussed it should be this. I've renamed the function to OnTerminate to
clear things up.

[Fady Samuel, 2015-06-04 23:20:27]

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

[Fady Samuel, 2015-06-04 23:20:29]

So that I can follow your work, could you please explain what problem this is
trying to solve? I'm a bit lost in the implementation details. Thanks!

[sky, 2015-06-04 23:24:20]

On 2015/06/04 23:20:29, Fady Samuel wrote:
> So that I can follow your work, could you please explain what problem this is
> trying to solve? I'm a bit lost in the implementation details. Thanks!

1: HTMLViewerApplication (now HTMLDocumentApplicationDelegate) was rolling its
own Application implementation. This meant we couldn't use some of the shared
code there, such as application lifetime.
2: Setup exposed an ApplicationImpl. When you run html_viewer you actually end
up with n+1 apps, where n is the number of urls you load. Nearly all services
that are used should be scoped to the app representing the document being loaded
(HTMLDocumentApplicationDelegate). This fixes that. Separately we may start
getting things from the ServiceProviders exposed via the ViewManager, but that's
separate.

[jam, 2015-06-05 00:53:35]

lgtm

[sky, 2015-06-05 15:08:05]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2015-06-05 15:08:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1165013005/40001

[commit-bot: I haz the power, 2015-06-05 15:36:39]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-05 15:38:35]

Patchset 3 (id:??) landed as
https://crrev.com/15df477605e2afc0abf2b05074c8ecbd8ac28c29
Cr-Commit-Position: refs/heads/master@{#333050}