Add Mouse hover behavior to Mac Panels when minimized

Rename PanelMouseWatcherGtk into PanelMouseWatcherTimer. Added implementation of gfx::Point GetCursorScreenPoint() on Mac.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=102173

[Dmitry Titov, 2011-09-20 23:18:57]

Need another iteration before requesting formal review...

[Dmitry Titov, 2011-09-21 00:33:31]

Looks ready for review! Hooked up the test as well.
There are some possible cleanups for later, added TODOs since this Cl is already
getting big.

[prasadt, 2011-09-21 16:54:15]

lgtm

A few suggestions but nothing that should block the check-in.  Giving lgtm as I
may not be able to check my mail in a timely manner today.

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
File chrome/browser/ui/panels/native_panel.h (right):

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
chrome/browser/ui/panels/native_panel.h:90: // TODO(dimich) Remove this method,
there is PanelMouseWatcher::GetInstance().
We don't compile the implementation of GetInstance into all the platforms. 
Using that directly in the test will make you to choose between some not so
great alternatives:
1) Exclude panel_browsertest.cc on certain platforms
2) Add a .cc file for each platform just to include a dummy defintion.
3) Use #if in panel_mouse_watcher.h file to include a dummy definition for some
platforms.

Using PanelMouseWatcher::GetInstance() was what was causing the build break. 
Having this method in NativePanelTesting seemed to me the best alternative. 
Just FYI, not to say it can't be improved upon.

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_browser_window_cocoa.mm (right):

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browser_window_cocoa.mm:124: height = 3;  //
TODO(dimich) merge with GTK patch which defines it better.
You should be able to just use PanelManager::minimized_panel_height().

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browser_window_cocoa.mm:147: int spaceAbovePanel
= 3 * [controller_ titlebarHeightInScreeenCoordinates];
Any reason for different behavior on Mac?

I see couple of issues with this:
1) If user minimizes panels, presumably he's using something in full screen mode
and he many not want panels popping up when he hovers on some control at the
bottom of the app.  This is not far fetched if you consider for example, most
media apps which have controls at the bottom.
2) Some other app taking the space right above the panel may have its own
behavior when you hover and we could be covering whatever that app throws up
with the panel titlebar popping up.
3) Consistency across platforms for panel behavior. This may not be a biggie as
a feature but would be nice as it allows us to test all platforms in a
consistent way.

I'm sure you have some advantages in mind for doing this.  Just offering a few
downsides to weigh that against.

[jennb, 2011-09-21 17:48:19]

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
File chrome/browser/ui/panels/native_panel.h (right):

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
chrome/browser/ui/panels/native_panel.h:90: // TODO(dimich) Remove this method,
there is PanelMouseWatcher::GetInstance().
On 2011/09/21 16:54:15, prasadt wrote:
> We don't compile the implementation of GetInstance into all the platforms. 
> Using that directly in the test will make you to choose between some not so
> great alternatives:
> 1) Exclude panel_browsertest.cc on certain platforms
> 2) Add a .cc file for each platform just to include a dummy defintion.
> 3) Use #if in panel_mouse_watcher.h file to include a dummy definition for
some
> platforms.
> 
> Using PanelMouseWatcher::GetInstance() was what was causing the build break. 
> Having this method in NativePanelTesting seemed to me the best alternative. 
> Just FYI, not to say it can't be improved upon.

The build break was because there was no implementation of
PanelMouseWatcher::GetInstance on all platforms. Once we have that, it will be
safe to remove this. We're not there yet, of course.

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_browser_window_cocoa.mm (right):

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browser_window_cocoa.mm:146: // under the
minimized panel or somewhat above it.
Comment should also explain 'why'.

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browser_window_cocoa.mm:147: int spaceAbovePanel
= 3 * [controller_ titlebarHeightInScreeenCoordinates];
On 2011/09/21 16:54:15, prasadt wrote:
> Any reason for different behavior on Mac?
> 
> I see couple of issues with this:
> 1) If user minimizes panels, presumably he's using something in full screen
mode
> and he many not want panels popping up when he hovers on some control at the
> bottom of the app.  This is not far fetched if you consider for example, most
> media apps which have controls at the bottom.
> 2) Some other app taking the space right above the panel may have its own
> behavior when you hover and we could be covering whatever that app throws up
> with the panel titlebar popping up.
> 3) Consistency across platforms for panel behavior. This may not be a biggie
as
> a feature but would be nice as it allows us to test all platforms in a
> consistent way.
> 
> I'm sure you have some advantages in mind for doing this.  Just offering a few
> downsides to weigh that against.

#1 is a good point. I know the reason for this extra space is to avoid moving
the mouse into the Mac Dock to peek at minimized panels, but I have to agree
with Prasad's first point.

http://codereview.chromium.org/7981012/diff/4011/ui/gfx/screen_mac.mm
File ui/gfx/screen_mac.mm (right):

http://codereview.chromium.org/7981012/diff/4011/ui/gfx/screen_mac.mm#newcode15
ui/gfx/screen_mac.mm:15: NSScreen* screen = [[NSScreen screens]
objectAtIndex:0];
Is it safe to use screen 0 even if the mouse is on a different screen?

[Dmitry Titov, 2011-09-21 20:38:51]

Addressed feedback, run tests on Mac (the only delta since last try jobs was in
.mm file)

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
File chrome/browser/ui/panels/native_panel.h (right):

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
chrome/browser/ui/panels/native_panel.h:90: // TODO(dimich) Remove this method,
there is PanelMouseWatcher::GetInstance().
On 2011/09/21 17:48:19, jennb wrote:
> On 2011/09/21 16:54:15, prasadt wrote:
> > We don't compile the implementation of GetInstance into all the platforms. 
> > Using that directly in the test will make you to choose between some not so
> > great alternatives:
> > 1) Exclude panel_browsertest.cc on certain platforms
> > 2) Add a .cc file for each platform just to include a dummy defintion.
> > 3) Use #if in panel_mouse_watcher.h file to include a dummy definition for
> some
> > platforms.
> > 
> > Using PanelMouseWatcher::GetInstance() was what was causing the build break.

> > Having this method in NativePanelTesting seemed to me the best alternative. 
> > Just FYI, not to say it can't be improved upon.
> 
> The build break was because there was no implementation of
> PanelMouseWatcher::GetInstance on all platforms. Once we have that, it will be
> safe to remove this. We're not there yet, of course.

Indeed. With GetInstance() implemented on all platforms, we could just use that.
All Singleton-based classes are usually accessed as FooBar::GetInstance(), we
probably should do the same.

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_browser_window_cocoa.mm (right):

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browser_window_cocoa.mm:124: height = 3;  //
TODO(dimich) merge with GTK patch which defines it better.
On 2011/09/21 16:54:15, prasadt wrote:
> You should be able to just use PanelManager::minimized_panel_height().

Thanks, done.

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browser_window_cocoa.mm:146: // under the
minimized panel or somewhat above it.
On 2011/09/21 17:48:19, jennb wrote:
> Comment should also explain 'why'.

Removed the comment since the 'space above' is gone.

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_browser_window_cocoa.mm:147: int spaceAbovePanel
= 3 * [controller_ titlebarHeightInScreeenCoordinates];
On 2011/09/21 17:48:19, jennb wrote:
> On 2011/09/21 16:54:15, prasadt wrote:
> > Any reason for different behavior on Mac?
> > 
> > I see couple of issues with this:
> > 1) If user minimizes panels, presumably he's using something in full screen
> mode
> > and he many not want panels popping up when he hovers on some control at the
> > bottom of the app.  This is not far fetched if you consider for example,
most
> > media apps which have controls at the bottom.
> > 2) Some other app taking the space right above the panel may have its own
> > behavior when you hover and we could be covering whatever that app throws up
> > with the panel titlebar popping up.
> > 3) Consistency across platforms for panel behavior. This may not be a biggie
> as
> > a feature but would be nice as it allows us to test all platforms in a
> > consistent way.
> > 
> > I'm sure you have some advantages in mind for doing this.  Just offering a
few
> > downsides to weigh that against.
> 
> #1 is a good point. I know the reason for this extra space is to avoid moving
> the mouse into the Mac Dock to peek at minimized panels, but I have to agree
> with Prasad's first point.

I think Prasad has a good point indeed, mostly about users being hunting for
some UI at the bottom of the apps windows. Removed the 'hover space above', will
address interaction with Dock in other ways (as part of auto-hide behavior
perhaps).

[Dmitry Titov, 2011-09-21 21:20:07]

On 2011/09/21 17:48:19, jennb wrote:
>
http://codereview.chromium.org/7981012/diff/4011/ui/gfx/screen_mac.mm#newcode15
> ui/gfx/screen_mac.mm:15: NSScreen* screen = [[NSScreen screens]
> objectAtIndex:0];
> Is it safe to use screen 0 even if the mouse is on a different screen?

The screens in multimon scenario form a global coordinate system, with the (0,0)
at the low-left corner of the primare screen (screen 0) on Mac and at the
top-left corner of the primary screen everywhere else (see, for example, Windows
explanation: http://www.flounder.com/virtual_screen_coordinates.htm).
So, gfx adopted Windows-like coordinate system. To transform from OSX to gfx, we
need to flip it relative to primary monitor, irrespective to where the
coordinate actually is.

[jennb, 2011-09-21 21:21:26]

LGTM

http://codereview.chromium.org/7981012/diff/4014/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_mouse_watcher_timer.cc (right):

http://codereview.chromium.org/7981012/diff/4014/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_mouse_watcher_timer.cc:14: class
PanelMouseWatcherTimer : public PanelMouseWatcher {
Thanks for making this diff-able!

http://codereview.chromium.org/7981012/diff/4014/chrome/browser/ui/panels/pan...
File chrome/browser/ui/panels/panel_titlebar_view_cocoa.mm (right):

http://codereview.chromium.org/7981012/diff/4014/chrome/browser/ui/panels/pan...
chrome/browser/ui/panels/panel_titlebar_view_cocoa.mm:37: clickCount:clickCount
Curious why click count is needed now?

[prasadt1, 2011-09-21 21:33:15]

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
File chrome/browser/ui/panels/native_panel.h (right):

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
chrome/browser/ui/panels/native_panel.h:90: // TODO(dimich) Remove this method,
there is PanelMouseWatcher::GetInstance().
On 2011/09/21 20:38:51, Dmitry Titov wrote:
> On 2011/09/21 17:48:19, jennb wrote:
> > On 2011/09/21 16:54:15, prasadt wrote:
> > > We don't compile the implementation of GetInstance into all the platforms.

> > > Using that directly in the test will make you to choose between some not
so
> > > great alternatives:
> > > 1) Exclude panel_browsertest.cc on certain platforms
> > > 2) Add a .cc file for each platform just to include a dummy defintion.
> > > 3) Use #if in panel_mouse_watcher.h file to include a dummy definition for
> > some
> > > platforms.
> > > 
> > > Using PanelMouseWatcher::GetInstance() was what was causing the build
break.
> 
> > > Having this method in NativePanelTesting seemed to me the best
alternative. 
> > > Just FYI, not to say it can't be improved upon.
> > 
> > The build break was because there was no implementation of
> > PanelMouseWatcher::GetInstance on all platforms. Once we have that, it will
be
> > safe to remove this. We're not there yet, of course.
> 
> Indeed. With GetInstance() implemented on all platforms, we could just use
that.

True but "all platforms" seems to be a bit of a moving target. Do a
auto_hiding_desktop_bar* in the panels directory to see all the platforms that
Jian had to handle to build successfully. "aura" is one of them, I don't know
what exactly it is but we may not be supporting it for a while.

> All Singleton-based classes are usually accessed as FooBar::GetInstance(), we
> probably should do the same.

The platform specific implementations of these methods are only accessed from
platform specific code, so there isn't an issue of unresolved symbols. The
platforms that refer to the method would also implement it.

Our case is different because we're accessing this method wich has platform
specific implementations, from panel_browsertest.cc which is platform
independent i.e it gets compiled/linked on all platforms.  This means we need a
dummy implementation for every platform, even "aura" which we may not care
about.  Note that this requirement is not there for building chrome (as opposed
to browser_tests) where we access this method only from platform specific files.

Prasad

[Dmitry Titov, 2011-09-21 21:41:54]

On 2011/09/21 21:21:26, jennb wrote:
http://codereview.chromium.org/7981012/diff/4014/chrome/browser/ui/panels/pan...
> File chrome/browser/ui/panels/panel_titlebar_view_cocoa.mm (right):
> 
>
http://codereview.chromium.org/7981012/diff/4014/chrome/browser/ui/panels/pan...
> chrome/browser/ui/panels/panel_titlebar_view_cocoa.mm:37:
clickCount:clickCount
> Curious why click count is needed now?

It's because I'm implementing test NativePanelTesting API, and it operates via
Press/Release type of mouse methods, calling both in a sequence to 'construct' a
click. On OSX, the clickCount==1 indicates that mouseUp event actually is a
'click'. However, if the user hesitates for a long time and releases the mouse
after some threshold, the clickCount==0 and usual 'click' actions do not happen.
clickCount==2 signifies a doubleclick. 

I think the moment we want to differentiate clicks from just mouse release after
a while, we will need to add MouseReleaseClick method to NativePanelTesting, in
addition to Press/Release.

[Dmitry Titov, 2011-09-21 21:46:58]

On 2011/09/21 21:33:15, prasadt1 wrote:
>
http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
> File chrome/browser/ui/panels/native_panel.h (right):
> 
>
http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
> chrome/browser/ui/panels/native_panel.h:90: // TODO(dimich) Remove this
method,
> there is PanelMouseWatcher::GetInstance().
> On 2011/09/21 20:38:51, Dmitry Titov wrote:
> > On 2011/09/21 17:48:19, jennb wrote:
> > > On 2011/09/21 16:54:15, prasadt wrote:
> > > > We don't compile the implementation of GetInstance into all the
platforms.
> 
> > > > Using that directly in the test will make you to choose between some not
> so
> > > > great alternatives:
> > > > 1) Exclude panel_browsertest.cc on certain platforms
> > > > 2) Add a .cc file for each platform just to include a dummy defintion.
> > > > 3) Use #if in panel_mouse_watcher.h file to include a dummy definition
for
> > > some
> > > > platforms.
> > > > 
> > > > Using PanelMouseWatcher::GetInstance() was what was causing the build
> break.
> > 
> > > > Having this method in NativePanelTesting seemed to me the best
> alternative. 
> > > > Just FYI, not to say it can't be improved upon.
> > > 
> > > The build break was because there was no implementation of
> > > PanelMouseWatcher::GetInstance on all platforms. Once we have that, it
will
> be
> > > safe to remove this. We're not there yet, of course.
> > 
> > Indeed. With GetInstance() implemented on all platforms, we could just use
> that.
> 
> True but "all platforms" seems to be a bit of a moving target. Do a
> auto_hiding_desktop_bar* in the panels directory to see all the platforms that
> Jian had to handle to build successfully. "aura" is one of them, I don't know
> what exactly it is but we may not be supporting it for a while.
> 
> > All Singleton-based classes are usually accessed as FooBar::GetInstance(),
we
> > probably should do the same.
> 
> The platform specific implementations of these methods are only accessed from
> platform specific code, so there isn't an issue of unresolved symbols. The
> platforms that refer to the method would also implement it.
> 
> Our case is different because we're accessing this method wich has platform
> specific implementations, from panel_browsertest.cc which is platform
> independent i.e it gets compiled/linked on all platforms.  This means we need
a
> dummy implementation for every platform, even "aura" which we may not care
> about.  Note that this requirement is not there for building chrome (as
opposed
> to browser_tests) where we access this method only from platform specific
files.
> 
> Prasad

I think 'aura' is new 'views'. It's what Ben is building...

Is there a difference between PanelMouseWatcher::GetInstance() and
Panel::CreateNativePanel()? Both are called from platform-independent code, so
both should provide the necessary implementations.

It seems that if any Panel classes are compiled on a given platform at all, the
PanelMouseWatcher should have some implementation there... If I am not missing
something.

[Dmitry Titov, 2011-09-21 22:10:12]

You both gave me LGTM , but there were additional quesitons, I feel I've
answered questions, so I'm landing it. If I didn't satisfy you, please raise the
issue and I'll fix it in subsequent patch.

[jennb, 2011-09-21 22:11:30]

LGTM++

On Wed, Sep 21, 2011 at 3:10 PM, <dimich@chromium.org> wrote:

> You both gave me LGTM , but there were additional quesitons, I feel I've
> answered questions, so I'm landing it. If I didn't satisfy you, please
> raise the
> issue and I'll fix it in subsequent patch.
>
>
>
http://codereview.chromium.**org/7981012/<http://codereview.chromium.org/7981...
>

[dcheng, 2011-09-21 22:12:41]

Drive-by comment.

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
File chrome/browser/ui/panels/native_panel.h (right):

http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
chrome/browser/ui/panels/native_panel.h:90: // TODO(dimich) Remove this method,
there is PanelMouseWatcher::GetInstance().
On 2011/09/21 21:33:15, prasadt1 wrote:
> On 2011/09/21 20:38:51, Dmitry Titov wrote:
> > On 2011/09/21 17:48:19, jennb wrote:
> > > On 2011/09/21 16:54:15, prasadt wrote:
> > > > We don't compile the implementation of GetInstance into all the
platforms.
> 
> > > > Using that directly in the test will make you to choose between some not
> so
> > > > great alternatives:
> > > > 1) Exclude panel_browsertest.cc on certain platforms
> > > > 2) Add a .cc file for each platform just to include a dummy defintion.
> > > > 3) Use #if in panel_mouse_watcher.h file to include a dummy definition
for
> > > some
> > > > platforms.
> > > > 
> > > > Using PanelMouseWatcher::GetInstance() was what was causing the build
> break.
> > 
> > > > Having this method in NativePanelTesting seemed to me the best
> alternative. 
> > > > Just FYI, not to say it can't be improved upon.
> > > 
> > > The build break was because there was no implementation of
> > > PanelMouseWatcher::GetInstance on all platforms. Once we have that, it
will
> be
> > > safe to remove this. We're not there yet, of course.
> > 
> > Indeed. With GetInstance() implemented on all platforms, we could just use
> that.
> 
> True but "all platforms" seems to be a bit of a moving target. Do a
> auto_hiding_desktop_bar* in the panels directory to see all the platforms that
> Jian had to handle to build successfully. "aura" is one of them, I don't know
> what exactly it is but we may not be supporting it for a while.
> 
> > All Singleton-based classes are usually accessed as FooBar::GetInstance(),
we
> > probably should do the same.
> 
> The platform specific implementations of these methods are only accessed from
> platform specific code, so there isn't an issue of unresolved symbols. The
> platforms that refer to the method would also implement it.
> 
> Our case is different because we're accessing this method wich has platform
> specific implementations, from panel_browsertest.cc which is platform
> independent i.e it gets compiled/linked on all platforms.  This means we need
a
> dummy implementation for every platform, even "aura" which we may not care
> about.  Note that this requirement is not there for building chrome (as
opposed
> to browser_tests) where we access this method only from platform specific
files.
> 
> Prasad

Just my two cents, but testing in general seems like one of those things we need
to think about a better story and probably do some refactoring. We have
essentially two different frameworks for testing panels (one Windows views
specific, one platform independent but written from a primarily GTK
point-of-view). The two approaches differ in several ways:
- views uses MessageLoop, browser test uses MessageLoopForUI.
- views prefers to pump the message loop directly in the test, while browser
test abstracts some of that into the testing interface (this means it's not
actually safe to use PanelMouseWatcher's methods directly in the browser test,
you need to go through NativePanelTesting)
- The set of hooks and entry points into the interfaces are somewhat different.

It just seems like there's room for consolidation here.

[prasadt, 2011-09-21 22:15:56]

On 2011/09/21 21:46:58, Dmitry Titov wrote:
> On 2011/09/21 21:33:15, prasadt1 wrote:
> >
>
http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
> > File chrome/browser/ui/panels/native_panel.h (right):
> > 
> >
>
http://codereview.chromium.org/7981012/diff/4011/chrome/browser/ui/panels/nat...
> > chrome/browser/ui/panels/native_panel.h:90: // TODO(dimich) Remove this
> method,
> > there is PanelMouseWatcher::GetInstance().
> > On 2011/09/21 20:38:51, Dmitry Titov wrote:
> > > On 2011/09/21 17:48:19, jennb wrote:
> > > > On 2011/09/21 16:54:15, prasadt wrote:
> > > > > We don't compile the implementation of GetInstance into all the
> platforms.
> > 
> > > > > Using that directly in the test will make you to choose between some
not
> > so
> > > > > great alternatives:
> > > > > 1) Exclude panel_browsertest.cc on certain platforms
> > > > > 2) Add a .cc file for each platform just to include a dummy defintion.
> > > > > 3) Use #if in panel_mouse_watcher.h file to include a dummy definition
> for
> > > > some
> > > > > platforms.
> > > > > 
> > > > > Using PanelMouseWatcher::GetInstance() was what was causing the build
> > break.
> > > 
> > > > > Having this method in NativePanelTesting seemed to me the best
> > alternative. 
> > > > > Just FYI, not to say it can't be improved upon.
> > > > 
> > > > The build break was because there was no implementation of
> > > > PanelMouseWatcher::GetInstance on all platforms. Once we have that, it
> will
> > be
> > > > safe to remove this. We're not there yet, of course.
> > > 
> > > Indeed. With GetInstance() implemented on all platforms, we could just use
> > that.
> > 
> > True but "all platforms" seems to be a bit of a moving target. Do a
> > auto_hiding_desktop_bar* in the panels directory to see all the platforms
that
> > Jian had to handle to build successfully. "aura" is one of them, I don't
know
> > what exactly it is but we may not be supporting it for a while.
> > 
> > > All Singleton-based classes are usually accessed as FooBar::GetInstance(),
> we
> > > probably should do the same.
> > 
> > The platform specific implementations of these methods are only accessed
from
> > platform specific code, so there isn't an issue of unresolved symbols. The
> > platforms that refer to the method would also implement it.
> > 
> > Our case is different because we're accessing this method wich has platform
> > specific implementations, from panel_browsertest.cc which is platform
> > independent i.e it gets compiled/linked on all platforms.  This means we
need
> a
> > dummy implementation for every platform, even "aura" which we may not care
> > about.  Note that this requirement is not there for building chrome (as
> opposed
> > to browser_tests) where we access this method only from platform specific
> files.
> > 
> > Prasad
> 
> I think 'aura' is new 'views'. It's what Ben is building...
> 
> Is there a difference between PanelMouseWatcher::GetInstance() and
> Panel::CreateNativePanel()? Both are called from platform-independent code, so
> both should provide the necessary implementations.

No.  There isn't any difference between the two.  I thought this is unique but I
guess we got this covered already.

> It seems that if any Panel classes are compiled on a given platform at all,
the
> PanelMouseWatcher should have some implementation there... If I am not missing
> something.

May be I'm mis-guided in thinking of aura as a "different" platform.  Any
platform that builds one of our NativePanel implementations should build one of
the PanelMouseWatcher implementation, eventually.  I thought, likely
incorrectly, that aura is an example of a platform where none of our three
NativePanel implementations gets built.

Just to make sure you're not waiting, you already got my lgtm.

Prasad