[Athena] minimum impl to add screen/background and test windows

[Athena] minimum impl to add screen/background and test windows

This sets the standard directory structure for Athena.

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

[oshima, 2014-05-16 19:32:01]

Still WIP, but want to get feedback on the structure of the directory/library.

I first looked into making sub directories components but that requires separate
xxx_export for each directory, which seems to be overkill. Instead, this CL
simply uses DEPS and public directory to control dependency. Is this reasonable?
I also wonder if we should have athena/public/xxx, instead of athena/xxx/public.
Please let me know.

[Albert Bodenhamer, 2014-05-16 20:11:24]

Thanks for putting this together.

Obviously, a lot of this is going to be boilerplate and placeholder stuff, but
do you think it's worth writing a rough DD and having a quick chat about how the
pieces fit together?

https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.cc
File athena/main/main_util.cc (right):

https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.c...
athena/main/main_util.cc:1: // Copyright 2014 The Chromium Authors. All rights
reserved.
Looks like main_util is just placeholder stuff?

[oshima, 2014-05-16 20:12:25]

On 2014/05/16 20:11:24, Albert Bodenhamer wrote:
> Thanks for putting this together.
> 
> Obviously, a lot of this is going to be boilerplate and placeholder stuff, but
> do you think it's worth writing a rough DD and having a quick chat about how
the
> pieces fit together?

Yes, I'm planning to write a DD/eng guide for athena, but wanted to hear you
guys opinion/feedback about theis.

> 
> https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.cc
> File athena/main/main_util.cc (right):
> 
>
https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.c...
> athena/main/main_util.cc:1: // Copyright 2014 The Chromium Authors. All rights
> reserved.
> Looks like main_util is just placeholder stuff?

[oshima, 2014-05-16 20:14:13]

https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.cc
File athena/main/main_util.cc (right):

https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.c...
athena/main/main_util.cc:1: // Copyright 2014 The Chromium Authors. All rights
reserved.
On 2014/05/16 20:11:24, Albert Bodenhamer wrote:
> Looks like main_util is just placeholder stuff?

Yes, this is just a dummy impl until we have real one. Do you want me to rename
something more clear?

[Ben Goodger (Google), 2014-05-16 20:16:47]

This layout lgtm. Nice deps files. As you lay out components I agree with Albert
it'd be good to sketch out a diagram showing the layercake.

https://codereview.chromium.org/287163009/diff/100001/athena/athena.gyp
File athena/athena.gyp (right):

https://codereview.chromium.org/287163009/diff/100001/athena/athena.gyp#newco...
athena/athena.gyp:23: # All .cc, .h under ash, except unittests
athena :-)

[Albert Bodenhamer, 2014-05-16 20:22:20]

To answer the original questions from the review description:
I think using DEPS and a public directory is fine.
I have a slight preference for athena/public/xxx as it gives an obvious place to
look for public interfaces.

https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.cc
File athena/main/main_util.cc (right):

https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.c...
athena/main/main_util.cc:1: // Copyright 2014 The Chromium Authors. All rights
reserved.
On 2014/05/16 20:14:13, oshima wrote:
> On 2014/05/16 20:11:24, Albert Bodenhamer wrote:
> > Looks like main_util is just placeholder stuff?
> 
> Yes, this is just a dummy impl until we have real one. Do you want me to
rename
> something more clear?

Perhaps. When I see a file named "main_util.h/cc" I get the impression that
there are important pieces there.

Maybe just call it "dummy_app"?

[oshima, 2014-05-16 20:52:19]

On 2014/05/16 20:22:20, Albert Bodenhamer wrote:
> To answer the original questions from the review description:
> I think using DEPS and a public directory is fine.
> I have a slight preference for athena/public/xxx as it gives an obvious place
to
> look for public interfaces.

These are public interfaces for code inside Athena. I now feel that I want to
keep athena/public for external
components, and use xxx/public for communication between athena modules. Does
this sound ok?

> 
> https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.cc
> File athena/main/main_util.cc (right):
> 
>
https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.c...
> athena/main/main_util.cc:1: // Copyright 2014 The Chromium Authors. All rights
> reserved.
> On 2014/05/16 20:14:13, oshima wrote:
> > On 2014/05/16 20:11:24, Albert Bodenhamer wrote:
> > > Looks like main_util is just placeholder stuff?
> > 
> > Yes, this is just a dummy impl until we have real one. Do you want me to
> rename
> > something more clear?
> 
> Perhaps. When I see a file named "main_util.h/cc" I get the impression that
> there are important pieces there.
> 
> Maybe just call it "dummy_app"?

Ok, I'll rename to something more obvious.

[oshima, 2014-05-16 22:26:33]

Hi, it's ready for review. PTAL.

https://codereview.chromium.org/287163009/diff/100001/athena/athena.gyp
File athena/athena.gyp (right):

https://codereview.chromium.org/287163009/diff/100001/athena/athena.gyp#newco...
athena/athena.gyp:23: # All .cc, .h under ash, except unittests
On 2014/05/16 20:16:47, Ben Goodger (Google) wrote:
> athena :-)

Done. :)

https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.cc
File athena/main/main_util.cc (right):

https://codereview.chromium.org/287163009/diff/100001/athena/main/main_util.c...
athena/main/main_util.cc:1: // Copyright 2014 The Chromium Authors. All rights
reserved.
On 2014/05/16 20:22:20, Albert Bodenhamer wrote:
> On 2014/05/16 20:14:13, oshima wrote:
> > On 2014/05/16 20:11:24, Albert Bodenhamer wrote:
> > > Looks like main_util is just placeholder stuff?
> > 
> > Yes, this is just a dummy impl until we have real one. Do you want me to
> rename
> > something more clear?
> 
> Perhaps. When I see a file named "main_util.h/cc" I get the impression that
> there are important pieces there.
> 
> Maybe just call it "dummy_app"?

renamed to just placeholder.h/cc as it does a bit more than app.

[Ben Goodger (Google), 2014-05-19 18:29:44]

lgtm

[chromium-reviews, 2014-05-19 18:58:39]

lgtm

On Mon May 19 2014 at 11:29:45 AM, <ben@chromium.org> wrote:

> lgtm
>
>
>
> https://codereview.chromium.org/287163009/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[oshima, 2014-05-19 19:02:05]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-05-19 19:03:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/oshima@chromium.org/287163009/150001

[commit-bot: I haz the power, 2014-05-20 01:08:09]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-20 04:12:32]

Change committed as 271561