Fix missing mandoline deps.

Fix missing mandoline deps.

Building "mandoline" should build html_viewer and ui_test.pak.

Committed: https://crrev.com/c7f40bc3252e97e62cb05d3e9e4e6b286497e0ec
Cr-Commit-Position: refs/heads/master@{#330759}

[jam, 2015-05-15 05:52:14]

jam@chromium.org changed reviewers:
+ sky@chromium.org

[jam, 2015-05-15 15:54:40]

jam@chromium.org changed reviewers:
+ msw@chromium.org
- sky@chromium.org

[jam, 2015-05-15 15:54:40]

->msw

[msw, 2015-05-15 16:53:10]

https://codereview.chromium.org/1128023007/diff/1/mandoline/app/BUILD.gn
File mandoline/app/BUILD.gn (left):

https://codereview.chromium.org/1128023007/diff/1/mandoline/app/BUILD.gn#oldc...
mandoline/app/BUILD.gn:19: "//components/html_viewer",
Why does moving this to mandoline/app/desktop help?
If we need html_viewer for android and desktop, isn't this right?

[jam, 2015-05-18 14:21:51]

https://codereview.chromium.org/1128023007/diff/1/mandoline/app/BUILD.gn
File mandoline/app/BUILD.gn (left):

https://codereview.chromium.org/1128023007/diff/1/mandoline/app/BUILD.gn#oldc...
mandoline/app/BUILD.gn:19: "//components/html_viewer",
On 2015/05/15 16:53:10, msw wrote:
> Why does moving this to mandoline/app/desktop help?
> If we need html_viewer for android and desktop, isn't this right?

the issue is that one when builds "mandoline", that builds the
mandoline/mandoline_apk targets in mandoline/app/{android,desktop}. the android
apk depended on html_viewer already. the desktop executable target didn't. they
don't depend on mandoline/app group.

[msw, 2015-05-18 18:04:42]

https://codereview.chromium.org/1128023007/diff/1/mandoline/app/BUILD.gn
File mandoline/app/BUILD.gn (left):

https://codereview.chromium.org/1128023007/diff/1/mandoline/app/BUILD.gn#oldc...
mandoline/app/BUILD.gn:19: "//components/html_viewer",
On 2015/05/18 14:21:51, jam wrote:
> On 2015/05/15 16:53:10, msw wrote:
> > Why does moving this to mandoline/app/desktop help?
> > If we need html_viewer for android and desktop, isn't this right?
> 
> the issue is that one when builds "mandoline", that builds the
> mandoline/mandoline_apk targets in mandoline/app/{android,desktop}. the
android
> apk depended on html_viewer already. the desktop executable target didn't.
they
> don't depend on mandoline/app group.

Ah ok, then shouldn't we move these other data deps to the mandoline targets as
well? Building "mandoline" from a clean output dir should suffice. I think it'd
be nice if this "app" target itself were renamed to be a cross-platform
"mandoline" target, listing the shared desktop/android deps and the
platform-specific executable target.

[jam, 2015-05-20 15:45:28]

https://codereview.chromium.org/1128023007/diff/1/mandoline/app/BUILD.gn
File mandoline/app/BUILD.gn (left):

https://codereview.chromium.org/1128023007/diff/1/mandoline/app/BUILD.gn#oldc...
mandoline/app/BUILD.gn:19: "//components/html_viewer",
On 2015/05/18 18:04:42, msw wrote:
> On 2015/05/18 14:21:51, jam wrote:
> > On 2015/05/15 16:53:10, msw wrote:
> > > Why does moving this to mandoline/app/desktop help?
> > > If we need html_viewer for android and desktop, isn't this right?
> > 
> > the issue is that one when builds "mandoline", that builds the
> > mandoline/mandoline_apk targets in mandoline/app/{android,desktop}. the
> android
> > apk depended on html_viewer already. the desktop executable target didn't.
> they
> > don't depend on mandoline/app group.
> 
> Ah ok, then shouldn't we move these other data deps to the mandoline targets
as
> well? Building "mandoline" from a clean output dir should suffice. I think
it'd
> be nice if this "app" target itself were renamed to be a cross-platform
> "mandoline" target, listing the shared desktop/android deps and the
> platform-specific executable target.

I don't know why exactly this app group was added. we should figure that out
asynchronously, but it seems we should land this now because we need building
"mandoline" to work correctly.

i can verify that building mandoline alone with this change fixes things.

[msw, 2015-05-20 16:59:07]

ok, lgtm

[jam, 2015-05-20 17:03:34]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2015-05-20 17:04:05]

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

[commit-bot: I haz the power, 2015-05-20 17:30:51]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-05-20 17:31:37]

Patchset 1 (id:??) landed as
https://crrev.com/c7f40bc3252e97e62cb05d3e9e4e6b286497e0ec
Cr-Commit-Position: refs/heads/master@{#330759}