MacViewsBrowser: Get browser_tests linking

MacViewsBrowser: Get browser_tests linking

Linking currently fails because mac_views_browser=1 doesn't build the
Cocoa browser, but the Cocoa tests are still included.

Introduce a new gyp variable to handle this and pick either the views
browser or Cocoa browser tests. A similar thing was done for unit_tests
in r365949.

BUG=575036
Committed: https://crrev.com/f75d97e7c5f8942f70b2fe2a5dfe8e9bb6b709b3
Cr-Commit-Position: refs/heads/master@{#373679}

[tapted, 2016-01-27 02:37:39]

Description was changed from

==========
MacViewsBrowser: Get browser_tests linking

Linking currently fails because mac_views_browser=1 doesn't build the
Cocoa browser, but the Cocoa tests are still included.

Introduce a new gyp variable to handle this.

BUG=575036
==========

to

==========
MacViewsBrowser: Get browser_tests linking

Linking currently fails because mac_views_browser=1 doesn't build the
Cocoa browser, but the Cocoa tests are still included.

Introduce a new gyp variable to handle this and pick either the views
browser or Cocoa browser tests.

BUG=575036
==========

[tapted, 2016-01-27 02:37:40]

tapted@chromium.org changed reviewers:
+ mgiuca@chromium.org

[tapted, 2016-01-27 02:38:34]

Hi Matt, could you please review? See if you agree with what I'm doing for
custom_launcher_page_browsertest_views.cc

https://codereview.chromium.org/1632323003/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1632323003/diff/1/chrome/chrome_tests.gypi#ne...
chrome/chrome_tests.gypi:672:
'browser/apps/custom_launcher_page_browsertest_views.cc',
This doesn't compile on Mac because the ui::test::EventGenerator expects to
receive a ui::EventTarget for some methods, and on Mac we pass an NSWindow
rather than an aura::Window. I could work around that with #ifdefs or something,
but really only ChromeOS cares about these tests, and that's unlikely to change,
so I went with this approach.

[Matt Giuca, 2016-01-29 02:07:03]

https://codereview.chromium.org/1632323003/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1632323003/diff/1/chrome/chrome_tests.gypi#ne...
chrome/chrome_tests.gypi:642: 'chrome_browser_tests_views_non_mac_sources': [
Maybe this should get renamed now that it can be included on Mac?

Definitely should at least update the comment.

https://codereview.chromium.org/1632323003/diff/1/chrome/chrome_tests.gypi#ne...
chrome/chrome_tests.gypi:672:
'browser/apps/custom_launcher_page_browsertest_views.cc',
On 2016/01/27 02:38:34, tapted wrote:
> This doesn't compile on Mac because the ui::test::EventGenerator expects to
> receive a ui::EventTarget for some methods, and on Mac we pass an NSWindow
> rather than an aura::Window. I could work around that with #ifdefs or
something,
> but really only ChromeOS cares about these tests, and that's unlikely to
change,
> so I went with this approach.

Acknowledged.

[tapted, 2016-01-29 03:27:45]

https://codereview.chromium.org/1632323003/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1632323003/diff/1/chrome/chrome_tests.gypi#ne...
chrome/chrome_tests.gypi:642: 'chrome_browser_tests_views_non_mac_sources': [
On 2016/01/29 02:07:03, Matt Giuca wrote:
> Maybe this should get renamed now that it can be included on Mac?
> 
> Definitely should at least update the comment.

Done. Updated comment, but kept the "non_mac" bit since it's easier to
understand than this views-browser thing (i.e. as far as a current Chrome
release is concerned, they are still non_mac)

[Matt Giuca, 2016-01-29 04:20:43]

lgtm

[tapted, 2016-01-29 09:24:11]

tapted@chromium.org changed reviewers:
+ ellyjones@chromium.org, thakis@chromium.org

[tapted, 2016-01-29 09:24:21]

tapted@chromium.org changed required reviewers:
+ thakis@chromium.org

[tapted, 2016-01-29 09:38:05]

Description was changed from

==========
MacViewsBrowser: Get browser_tests linking

Linking currently fails because mac_views_browser=1 doesn't build the
Cocoa browser, but the Cocoa tests are still included.

Introduce a new gyp variable to handle this and pick either the views
browser or Cocoa browser tests.

BUG=575036
==========

to

==========
MacViewsBrowser: Get browser_tests linking

Linking currently fails because mac_views_browser=1 doesn't build the
Cocoa browser, but the Cocoa tests are still included.

Introduce a new gyp variable to handle this and pick either the views
browser or Cocoa browser tests. A similar thing was done for unit_tests
in r365949.

BUG=575036
==========

[tapted, 2016-01-29 09:38:30]

+thakis for OWNERS

and Elly - please take a look too - this is approach is a bit more ambitious but
it will let us maximize coverage on the bot. But note this doesn't do anything
for interactive_ui_tests -- they might still need that BUILDFLAG in
http://crrrev.com/1647563002 unless we split up those files.

[tapted, 2016-02-02 11:57:45]

elly/nico: ping?

[Nico, 2016-02-02 22:12:27]

stampy lgtm

https://codereview.chromium.org/1632323003/diff/20001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1632323003/diff/20001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:587:
'browser/ui/cocoa/applescript/browsercrapplication+applescript_test.mm',
does views have applescript browser tests? if the views switch happens, we
shouldn't blindly delete this block, there's possibly test coverage here that
isn't anywhere else...

[tapted, 2016-02-02 22:30:25]

(I think elly is ooo today, but this isn't terribly urgent)

https://codereview.chromium.org/1632323003/diff/20001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1632323003/diff/20001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:587:
'browser/ui/cocoa/applescript/browsercrapplication+applescript_test.mm',
On 2016/02/02 22:12:27, Nico wrote:
> does views have applescript browser tests? if the views switch happens, we
> shouldn't blindly delete this block, there's possibly test coverage here that
> isn't anywhere else...

Applescript isn't linked in yet for mac_views_browser (i.e. for the non-test
stuff in chrome_browser_ui_cocoa_sources), but it's definitely on the list of
requirements for the views browser before we think about releasing it. So yeah,
there is stuff under browser/ui/cocoa that needs to stay, and I'll be sure to
keep their tests as well.

And rather than switching to mac_views_browser=1, I think the approach will be
to instead migrate things gradually into the mac_views_browser=0 build and phase
out mac_views_browser in GYP. So there shouldn't be a sudden moment where this
block gets culled.

[Nico, 2016-02-02 22:51:49]

https://codereview.chromium.org/1632323003/diff/20001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/1632323003/diff/20001/chrome/chrome_tests.gyp...
chrome/chrome_tests.gypi:587:
'browser/ui/cocoa/applescript/browsercrapplication+applescript_test.mm',
Ah, that sounds like a good strategy. Thanks!

[Elly Fong-Jones, 2016-02-04 13:43:28]

lgtm from me :)

[tapted, 2016-02-04 23:11:37]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2016-02-04 23:13:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1632323003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1632323003/20001

[commit-bot: I haz the power, 2016-02-05 00:22:49]

Description was changed from

==========
MacViewsBrowser: Get browser_tests linking

Linking currently fails because mac_views_browser=1 doesn't build the
Cocoa browser, but the Cocoa tests are still included.

Introduce a new gyp variable to handle this and pick either the views
browser or Cocoa browser tests. A similar thing was done for unit_tests
in r365949.

BUG=575036
==========

to

==========
MacViewsBrowser: Get browser_tests linking

Linking currently fails because mac_views_browser=1 doesn't build the
Cocoa browser, but the Cocoa tests are still included.

Introduce a new gyp variable to handle this and pick either the views
browser or Cocoa browser tests. A similar thing was done for unit_tests
in r365949.

BUG=575036
==========

[commit-bot: I haz the power, 2016-02-05 00:22:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-05 00:23:45]

Description was changed from

==========
MacViewsBrowser: Get browser_tests linking

Linking currently fails because mac_views_browser=1 doesn't build the
Cocoa browser, but the Cocoa tests are still included.

Introduce a new gyp variable to handle this and pick either the views
browser or Cocoa browser tests. A similar thing was done for unit_tests
in r365949.

BUG=575036
==========

to

==========
MacViewsBrowser: Get browser_tests linking

Linking currently fails because mac_views_browser=1 doesn't build the
Cocoa browser, but the Cocoa tests are still included.

Introduce a new gyp variable to handle this and pick either the views
browser or Cocoa browser tests. A similar thing was done for unit_tests
in r365949.

BUG=575036

Committed: https://crrev.com/f75d97e7c5f8942f70b2fe2a5dfe8e9bb6b709b3
Cr-Commit-Position: refs/heads/master@{#373679}
==========

[commit-bot: I haz the power, 2016-02-05 00:23:46]

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