[MacViews] Add NativeAppWindowViewsMac and port HideWithApp behavior from NativeAppWindowCocoa.

[MacViews] Add NativeAppWindowViewsMac and port HideWithApp behavior from NativeAppWindowCocoa.

Test coverage is provided by native_app_window_cocoa_browsertest.mm
which now uses whichever implementation of NativeAppWindow is
instantiated.

Added a new sources list to minimize having to change BUILD.gn when
more files are added.

BUG=459877
Committed: https://crrev.com/edb06b840e75515f0ca9d9284fdbfe596ac939fd
Cr-Commit-Position: refs/heads/master@{#319153}

[jackhou1, 2015-03-04 02:09:40]

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

[jackhou1, 2015-03-04 02:09:41]

tapted, could you take a look?

[tapted, 2015-03-04 02:27:33]

+mac-views-reviews :)

Can we just run the tests in NativeAppWindowCocoaBrowserTest ?

At a glance, there don't actually seem to be any checks that require the
native_app_window_cocoa.h #include at the top of the file. Changing that to 

`#include "extensions/browser/app_window/native_app_window.h"`

should allow the one test file to cater for either, depending which version of
CreateNativeAppWindowImpl is linked into the test binary.

(if that works some of the comments below might become obsolete)

https://codereview.chromium.org/975043003/diff/1/chrome/browser/ui/views/apps...
File
chrome/browser/ui/views/apps/chrome_native_app_window_views_mac_browsertest.mm
(right):

https://codereview.chromium.org/975043003/diff/1/chrome/browser/ui/views/apps...
chrome/browser/ui/views/apps/chrome_native_app_window_views_mac_browsertest.mm:61:
EXPECT_TRUE(ns_window);
indenting is weird here

https://codereview.chromium.org/975043003/diff/1/chrome/chrome_browser_ui.gypi
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/975043003/diff/1/chrome/chrome_browser_ui.gyp...
chrome/chrome_browser_ui.gypi:2861:
#'browser/ui/cocoa/bookmarks/bookmark_drag_drop_cocoa.mm',
I guess this is commented out in your upstream? it might cause a conflict here..

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

https://codereview.chromium.org/975043003/diff/1/chrome/chrome_tests.gypi#new...
chrome/chrome_tests.gypi:2331: ['toolkit_views==1 and OS=="mac" and
mac_views_browser==1', {
nit: just check mac_views_browser?

https://codereview.chromium.org/975043003/diff/1/chrome/chrome_tests.gypi#new...
chrome/chrome_tests.gypi:2333:
'browser/ui/views/apps/chrome_native_app_window_views_mac_browsertest.mm',
Hm. I think this needs a new variable, otherwise each change needs a
corresponding gn change as well (a variable will let us do just one gn change).
Since it will just be macviews stuff, I don't think an extra variable will make
things more confusing.

Then the variable can have a TODO like ~ "Move items from this list to
chrome_browser_tests_views_sources when they migrate from mac_views_browser to a
chrome://flag"

The non-test file list should have a variable too -- but just for the stuff
being added (i.e. a variable won't work for the `sources!` keys)

[jackhou1, 2015-03-04 03:34:55]

https://codereview.chromium.org/975043003/diff/1/chrome/browser/ui/views/apps...
File
chrome/browser/ui/views/apps/chrome_native_app_window_views_mac_browsertest.mm
(right):

https://codereview.chromium.org/975043003/diff/1/chrome/browser/ui/views/apps...
chrome/browser/ui/views/apps/chrome_native_app_window_views_mac_browsertest.mm:61:
EXPECT_TRUE(ns_window);
On 2015/03/04 02:27:33, tapted wrote:
> indenting is weird here

Just using native_app_window_cocoa_browsertest.mm now.

https://codereview.chromium.org/975043003/diff/1/chrome/chrome_browser_ui.gypi
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/975043003/diff/1/chrome/chrome_browser_ui.gyp...
chrome/chrome_browser_ui.gypi:2861:
#'browser/ui/cocoa/bookmarks/bookmark_drag_drop_cocoa.mm',
On 2015/03/04 02:27:33, tapted wrote:
> I guess this is commented out in your upstream? it might cause a conflict
here..

The trybots ran fine. But rebased anyway.

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

https://codereview.chromium.org/975043003/diff/1/chrome/chrome_tests.gypi#new...
chrome/chrome_tests.gypi:2331: ['toolkit_views==1 and OS=="mac" and
mac_views_browser==1', {
On 2015/03/04 02:27:33, tapted wrote:
> nit: just check mac_views_browser?

Removed.

https://codereview.chromium.org/975043003/diff/1/chrome/chrome_tests.gypi#new...
chrome/chrome_tests.gypi:2333:
'browser/ui/views/apps/chrome_native_app_window_views_mac_browsertest.mm',
On 2015/03/04 02:27:33, tapted wrote:
> Hm. I think this needs a new variable, otherwise each change needs a
> corresponding gn change as well (a variable will let us do just one gn
change).
> Since it will just be macviews stuff, I don't think an extra variable will
make
> things more confusing.
> 
> Then the variable can have a TODO like ~ "Move items from this list to
> chrome_browser_tests_views_sources when they migrate from mac_views_browser to
a
> chrome://flag"
> 
> The non-test file list should have a variable too -- but just for the stuff
> being added (i.e. a variable won't work for the `sources!` keys)

Done.

[tapted, 2015-03-04 04:13:34]

lgtm with the below.

Also a bit more in the CL description.

E.g. say that it has test coverage, and say why the gyp/gn changes are being
done the way they are.

https://codereview.chromium.org/975043003/diff/20001/chrome/chrome_browser_ui...
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/975043003/diff/20001/chrome/chrome_browser_ui...
chrome/chrome_browser_ui.gypi:2309: 'chrome_browser_ui_views_mac_sources': [
this name isn't quite right - I'd save `chrome_browser_ui_views_mac_sources` for
things we need only on mac to support the default build, rather than the build
behind a compile time flag. I guess there's nothing in there yet [ :o ]. Note
that that class of files might be achievable with _mac or .mm suffixes, but I
think GN wants to move away from that somewhat.

Maybe chrome_browser_ui_views_mac_experimental_sources ?

[jackhou1, 2015-03-04 04:37:13]

https://codereview.chromium.org/975043003/diff/20001/chrome/chrome_browser_ui...
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/975043003/diff/20001/chrome/chrome_browser_ui...
chrome/chrome_browser_ui.gypi:2309: 'chrome_browser_ui_views_mac_sources': [
On 2015/03/04 04:13:34, tapted wrote:
> this name isn't quite right - I'd save `chrome_browser_ui_views_mac_sources`
for
> things we need only on mac to support the default build, rather than the build
> behind a compile time flag. I guess there's nothing in there yet [ :o ]. Note
> that that class of files might be achievable with _mac or .mm suffixes, but I
> think GN wants to move away from that somewhat.
> 
> Maybe chrome_browser_ui_views_mac_experimental_sources ?

Done.

[jackhou1, 2015-03-04 06:24:31]

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

[jackhou1, 2015-03-04 06:24:31]

sky, please review for OWNERS of:

chrome/browser/ui/BUILD.gn

[sky, 2015-03-04 16:30:51]

LGTM

[jackhou1, 2015-03-04 22:56:23]

The CQ bit was checked by jackhou@chromium.org

[jackhou1, 2015-03-04 22:56:24]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/975043003/#ps40001 (title:
"Change gypi variable name.")

[commit-bot: I haz the power, 2015-03-04 22:57:33]

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

[commit-bot: I haz the power, 2015-03-04 23:01:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-04 23:02:22]

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