Minimize headless code that refers to aura.

Remove code and minimize references to aura as to facilitate refractoring needed for Mac headless
BUG=687407
Review-Url: https://codereview.chromium.org/2669693002
Cr-Commit-Position: refs/heads/master@{#449909}
Committed: https://chromium.googlesource.com/chromium/src/+/df183ec6ada211dda086eda5b3bb940f7802ca9f

[dvallet, 2017-02-01 02:30:32]

Description was changed from

==========
Remove code and minimize references to aura
BUG=687407
==========

to

==========
Remove code and minimize references to aura as to facilitate refractoring needed
for Mac
BUG=687407
==========

[dvallet, 2017-02-01 02:30:39]

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

[commit-bot: I haz the power, 2017-02-01 02:31:18]

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, 2017-02-01 04:09:40]

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

[commit-bot: I haz the power, 2017-02-01 04:09:41]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dvallet, 2017-02-01 23:52:42]

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

[commit-bot: I haz the power, 2017-02-01 23:53:44]

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, 2017-02-02 01:16:28]

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

[commit-bot: I haz the power, 2017-02-02 01:16:28]

Dry run: This issue passed the CQ dry run.

[dvallet, 2017-02-02 03:05:32]

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

[commit-bot: I haz the power, 2017-02-02 03:06:05]

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, 2017-02-02 06:15:13]

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

[commit-bot: I haz the power, 2017-02-02 06:15:14]

Dry run: This issue passed the CQ dry run.

[dvallet, 2017-02-07 03:47:58]

Description was changed from

==========
Remove code and minimize references to aura as to facilitate refractoring needed
for Mac
BUG=687407
==========

to

==========
Remove code and minimize references to aura as to facilitate refractoring needed
for Mac headless
BUG=687407
==========

[dvallet, 2017-02-07 03:47:58]

dvallet@chromium.org changed reviewers:
+ samistil@chromium.org

[dvallet, 2017-02-07 03:49:27]

This is an initial step to put all aura related code in a single file. 
I've also removed code that I don't see referred in the code or doesn't seem to
affect tests/headless behavior that have anything to do with Aura.

[dvallet, 2017-02-07 03:49:37]

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

[commit-bot: I haz the power, 2017-02-07 03:50:00]

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, 2017-02-07 04:23:07]

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

[commit-bot: I haz the power, 2017-02-07 04:23:08]

Dry run: This issue passed the CQ dry run.

[dvallet, 2017-02-07 21:50:06]

dvallet@chromium.org changed reviewers:
+ skyostil@chromium.org
- samistil@chromium.org

[dvallet, 2017-02-08 04:50:08]

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

[commit-bot: I haz the power, 2017-02-08 04:50:20]

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, 2017-02-08 06:06:44]

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

[commit-bot: I haz the power, 2017-02-08 06:06:44]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dvallet, 2017-02-09 00:16:58]

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

[commit-bot: I haz the power, 2017-02-09 00:17:57]

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, 2017-02-09 01:29:02]

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

[commit-bot: I haz the power, 2017-02-09 01:29:03]

Dry run: This issue passed the CQ dry run.

[dvallet, 2017-02-09 03:38:52]

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

[commit-bot: I haz the power, 2017-02-09 03:39:29]

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

[dvallet, 2017-02-09 06:13:27]

PTAL
if preferred I can hold on this with the changes that makes it compile on Mac,
but I thought it's easier this way.

[commit-bot: I haz the power, 2017-02-09 06:25:47]

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

[commit-bot: I haz the power, 2017-02-09 06:25:48]

Dry run: This issue passed the CQ dry run.

[Sami, 2017-02-09 18:07:47]

Description was changed from

==========
Remove code and minimize references to aura as to facilitate refractoring needed
for Mac headless
BUG=687407
==========

to

==========
Remove code and minimize references to aura as to facilitate refractoring needed
for Mac headless
BUG=687407
==========

[Sami, 2017-02-09 18:07:47]

skyostil@chromium.org changed reviewers:
+ eseckler@chromium.org

[Sami, 2017-02-09 18:08:22]

I think the platform split makes sense. We might want to introduce directories
or proper interfaces if it things become much more complex than this, but this
seems like a good starting point.

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_browser_impl.h (right):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_browser_impl.h:72: void PlatformSetContents(const
gfx::Size& initial_size,
nit: PlatformSetWebContents since that's what the object is typically called

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_screen.cc (left):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_screen.cc:37: aura::WindowTreeHost*
HeadlessScreen::CreateHostForPrimaryDisplay() {
Are we sure we don't need this code anymore? +Eric, do you remember if we added
it for a reason?

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_screen.cc (right):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_screen.cc:35: const gfx::Point& point) {
Should we be calling into the platform here?

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_window_tree_host.h (right):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_window_tree_host.h:24: gfx::NativeWindow
NativeWindow();
Not sure why we can't just use window()? This file will only be built on Aura,
and there Window == NativeWindow, right?

[Eric Seckler, 2017-02-09 18:29:39]

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_browser_impl.h (right):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_browser_impl.h:19: #include
"headless/lib/browser/headless_window_tree_host.h"
This reflects what Sami mentioned in headless_window_tree_host.h: I don't think
we can include/use HeadlessWindowTreeHost here, since it depends on aura?

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_browser_main_parts.cc (right):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_browser_main_parts.cc:16: void PlatformExit() {
Shall we move this into BrowserImpl as well - seems sensible to keep it in same
place as PlatformInitialize? (Or get rid of it, since it doesn't seem to be
necessary?)

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_screen.cc (left):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_screen.cc:37: aura::WindowTreeHost*
HeadlessScreen::CreateHostForPrimaryDisplay() {
On 2017/02/09 18:08:22, Sami wrote:
> Are we sure we don't need this code anymore? +Eric, do you remember if we
added
> it for a reason?

Hm, I think this code got here before my time... It looks like none of these
methods are called anywhere, so I guess we can get rid of it.
GetWindowAtScreenPoint is the only method that might need updating (and probably
is already broken at the moment).

[dvallet, 2017-02-10 03:19:32]

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

[commit-bot: I haz the power, 2017-02-10 03:20:04]

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

[dvallet, 2017-02-10 05:12:21]

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_browser_impl.h (right):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_browser_impl.h:19: #include
"headless/lib/browser/headless_window_tree_host.h"
On 2017/02/09 at 18:29:39, Eric Seckler wrote:
> This reflects what Sami mentioned in headless_window_tree_host.h: I don't
think we can include/use HeadlessWindowTreeHost here, since it depends on aura?

Done, I've added #if defined(USE_AURA) directives, similar to how it's done in
https://cs.chromium.org/chromium/src/content/shell/browser/shell.h

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_browser_impl.h:72: void PlatformSetContents(const
gfx::Size& initial_size,
On 2017/02/09 at 18:08:22, Sami wrote:
> nit: PlatformSetWebContents since that's what the object is typically called

Done

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_browser_main_parts.cc (right):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_browser_main_parts.cc:16: void PlatformExit() {
On 2017/02/09 at 18:29:39, Eric Seckler wrote:
> Shall we move this into BrowserImpl as well - seems sensible to keep it in
same place as PlatformInitialize? (Or get rid of it, since it doesn't seem to be
necessary?)

I'll clean it for now, afaik is not needed in Mac either

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_screen.cc (right):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_screen.cc:35: const gfx::Point& point) {
On 2017/02/09 at 18:08:22, Sami wrote:
> Should we be calling into the platform here?

I'll have to check if this is needed for Mac and if so I'll add it as platform.
Note the HeadlessScreen is only used in aura implementation and I haven't seen
an alternative used for Mac.

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
File headless/lib/browser/headless_window_tree_host.h (right):

https://codereview.chromium.org/2669693002/diff/120001/headless/lib/browser/h...
headless/lib/browser/headless_window_tree_host.h:24: gfx::NativeWindow
NativeWindow();
On 2017/02/09 at 18:08:22, Sami wrote:
> Not sure why we can't just use window()? This file will only be built on Aura,
and there Window == NativeWindow, right?

You are right, this came from an old refractoring.

[commit-bot: I haz the power, 2017-02-10 05:25:26]

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

[commit-bot: I haz the power, 2017-02-10 05:25:27]

Dry run: This issue passed the CQ dry run.

[Sami, 2017-02-10 13:53:12]

Thanks, lgtm with a few nits.

https://codereview.chromium.org/2669693002/diff/180001/headless/lib/browser/h...
File headless/lib/browser/headless_browser_impl.h (right):

https://codereview.chromium.org/2669693002/diff/180001/headless/lib/browser/h...
headless/lib/browser/headless_browser_impl.h:75: void
PlatformSetWebContents(const gfx::Size& initial_size,
Sorry for not noticing earlier, but something like PlatformInitializeWebContents
would be more appropriate since we can have several and they aren't really "set"
anywhere.

https://codereview.chromium.org/2669693002/diff/180001/headless/lib/browser/h...
File headless/lib/browser/headless_screen.cc (right):

https://codereview.chromium.org/2669693002/diff/180001/headless/lib/browser/h...
headless/lib/browser/headless_screen.cc:36: return nullptr;
Need reformatting? (try git cl format)

https://codereview.chromium.org/2669693002/diff/180001/headless/lib/browser/h...
File headless/lib/browser/headless_window_tree_host.h (right):

https://codereview.chromium.org/2669693002/diff/180001/headless/lib/browser/h...
headless/lib/browser/headless_window_tree_host.h:25: gfx::NativeWindow
GetTopWindowContainingPoint(const gfx::Point& point);
Looks like this isn't called from anywhere. Let's just remove it?

[dvallet, 2017-02-13 03:43:06]

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

[commit-bot: I haz the power, 2017-02-13 03:43:20]

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, 2017-02-13 04:45:24]

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

[commit-bot: I haz the power, 2017-02-13 04:45:25]

Dry run: This issue passed the CQ dry run.

[dvallet, 2017-02-13 04:46:11]

The CQ bit was checked by dvallet@chromium.org

[dvallet, 2017-02-13 04:46:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/2669693002/#ps200001
(title: "Remove GetTopWindowContainingPoint")

[commit-bot: I haz the power, 2017-02-13 04:46:31]

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, 2017-02-13 04:49:54]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1486961171173490,
"parent_rev": "42baa1e97512657b037aa3c6f7e0fa91b1ff31c2", "commit_rev":
"df183ec6ada211dda086eda5b3bb940f7802ca9f"}

[commit-bot: I haz the power, 2017-02-13 04:51:10]

Description was changed from

==========
Remove code and minimize references to aura as to facilitate refractoring needed
for Mac headless
BUG=687407
==========

to

==========
Remove code and minimize references to aura as to facilitate refractoring needed
for Mac headless
BUG=687407

Review-Url: https://codereview.chromium.org/2669693002
Cr-Commit-Position: refs/heads/master@{#449909}
Committed:
https://chromium.googlesource.com/chromium/src/+/df183ec6ada211dda086eda5b3bb...
==========

[commit-bot: I haz the power, 2017-02-13 04:51:11]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/df183ec6ada211dda086eda5b3bb...