[MacViews] Fix AccessiblePaneViewTest.SetPaneFocusAndRestore.

[MacViews] Fix AccessiblePaneViewTest.SetPaneFocusAndRestore.

Fake out key status to make Widget::Activate/IsActive work in the unit test.
If-def out the part that requires Widget::Deactivate as it's not supported on
Mac.

The test was also failing because Mac checks WidgetDelegate::CanActivate()
when activating whereas NativeWidgetAura defers this to `ShouldActivate`
which is only checked by wm::BaseFocusRules.

Also improve c/b/ui/test/scoped_fake_nswindow_main_status to handle
key status as well and move it to ui/base/test/.

BUG=378134
Committed: https://crrev.com/24869918ffc61b2a29e7a8b23d9120e13c3404fd
Cr-Commit-Position: refs/heads/master@{#342289}

[jackhou1, 2015-08-05 06:38:03]

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

[jackhou1, 2015-08-05 06:38:04]

tapted, PTAL

[tapted, 2015-08-05 07:07:18]

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
File ui/base/test/scoped_fake_nswindow_focus.h (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.h:5: #ifndef
CHROME_BROWSER_UI_TEST_SCOPED_FAKE_NSWINDOW_FOCUS_H_
UI_BASE_TEST_SCOPED_FAKE_NSWINDOW_FOCUS_H_

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.h:41: class ScopedFakeNSWindowKeyStatus
{
Do we need both of these? (what happens if we merge them into one class that
does both main and key status, since that's the way things Normally work). Tests
probably don't need to care....

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
File ui/base/test/scoped_fake_nswindow_focus.mm (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:17: void SetAndNotify(NSWindow**
state,
I'm undecided about these methods.

It might be be nicer if it returned |window|, and only do the DCHECK in the
ScopedFakeNSWindowMainStatus constructor. then do calls like

g_fake_main_window = SetAndNotify(window, NSWindowDidBecomeMainNotification);

g_fake_main_window = ClearAndNotify(g_fake_main_window,
NSWindowDidResignMainNotification);


Another option might be to lose the genericity and do

SetMain(NSWindow* window) {
  DCHECK_NE(!window, !g_fake_main_window);
  NSString* name = window ?
      NSWindowDidBecomeMainNotification : NSWindowDidResignMainNotification;
  [[NSNotificationCenter defaultCenter] postNotificationName:name
      object:window ? window : g_fake_main_window];
  g_fake_main_window = window;
}


But! all this becomes irrelevant if we can combine the two classes, and just
have one key+main window

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:35: }
blank line +
}  // namespace

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:50: @interface
IsKeyWindowDonorForWindow : NSObject
even if we can't merge the C++ classes, it should be fine to merge the donor
classes -- the swizzler doesn't care where they come from

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:59:
ClearAndNotify(&g_fake_key_window, NSWindowDidResignKeyNotification);
Before this, it should do

if (![window canBecomeKeyWindow])
  return;

But I think that will cause the test to fail.

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:123: widget_main->Activate();
I think this should fail since widget->CanActivate returns false, no?

[jackhou1, 2015-08-06 00:22:30]

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
File ui/base/test/scoped_fake_nswindow_focus.h (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.h:5: #ifndef
CHROME_BROWSER_UI_TEST_SCOPED_FAKE_NSWINDOW_FOCUS_H_
On 2015/08/05 07:07:17, tapted wrote:
> UI_BASE_TEST_SCOPED_FAKE_NSWINDOW_FOCUS_H_

Done.

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.h:41: class ScopedFakeNSWindowKeyStatus
{
On 2015/08/05 07:07:17, tapted wrote:
> Do we need both of these? (what happens if we merge them into one class that
> does both main and key status, since that's the way things Normally work).
Tests
> probably don't need to care....

Done.

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
File ui/base/test/scoped_fake_nswindow_focus.mm (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:17: void SetAndNotify(NSWindow**
state,
On 2015/08/05 07:07:17, tapted wrote:
> I'm undecided about these methods.
> 
> It might be be nicer if it returned |window|, and only do the DCHECK in the
> ScopedFakeNSWindowMainStatus constructor. then do calls like
> 
> g_fake_main_window = SetAndNotify(window, NSWindowDidBecomeMainNotification);
> 
> g_fake_main_window = ClearAndNotify(g_fake_main_window,
> NSWindowDidResignMainNotification);
> 
> 
> Another option might be to lose the genericity and do
> 
> SetMain(NSWindow* window) {
>   DCHECK_NE(!window, !g_fake_main_window);
>   NSString* name = window ?
>       NSWindowDidBecomeMainNotification : NSWindowDidResignMainNotification;
>   [[NSNotificationCenter defaultCenter] postNotificationName:name
>       object:window ? window : g_fake_main_window];
>   g_fake_main_window = window;
> }
> 
> 
> But! all this becomes irrelevant if we can combine the two classes, and just
> have one key+main window

Done.

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:35: }
On 2015/08/05 07:07:17, tapted wrote:
> blank line +
> }  // namespace

Done.

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:50: @interface
IsKeyWindowDonorForWindow : NSObject
On 2015/08/05 07:07:17, tapted wrote:
> even if we can't merge the C++ classes, it should be fine to merge the donor
> classes -- the swizzler doesn't care where they come from

Done.

https://codereview.chromium.org/1268293002/diff/30001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:59:
ClearAndNotify(&g_fake_key_window, NSWindowDidResignKeyNotification);
On 2015/08/05 07:07:17, tapted wrote:
> Before this, it should do
> 
> if (![window canBecomeKeyWindow])
>   return;
> 
> But I think that will cause the test to fail.

Yeah, that returns NO. If we really want to support this, we might have to
swizzle -[NSWindow canBecomeKeyWindow] to return YES for classes that don't
override it. But I don't think that's necessary.

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:123: widget_main->Activate();
On 2015/08/05 07:07:17, tapted wrote:
> I think this should fail since widget->CanActivate returns false, no?

CanActivate() returns false on the other platforms too, it doesn't seem to block
Activate().

[tapted, 2015-08-06 01:21:17]

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:123: widget_main->Activate();
On 2015/08/06 00:22:30, jackhou1 wrote:
> On 2015/08/05 07:07:17, tapted wrote:
> > I think this should fail since widget->CanActivate returns false, no?
> 
> CanActivate() returns false on the other platforms too, it doesn't seem to
block
> Activate().

I think this is a bug though. other platforms should obey CanActivate

https://codereview.chromium.org/1268293002/diff/50001/ui/base/test/scoped_fak...
File ui/base/test/scoped_fake_nswindow_focus.mm (right):

https://codereview.chromium.org/1268293002/diff/50001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:65: - (void)makeMainWindow {
just call [self makeKeyWindow]? Then it's clearer that the faker doesn't really
try to distinguish them,

https://codereview.chromium.org/1268293002/diff/50001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:81: new
base::mac::ScopedObjCClassSwizzler([NSWindow class],
a `using base::mac::ScopedObjCClassSwizzler` might convince clang-format to do a
tidier job here

[jackhou1, 2015-08-06 01:51:36]

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:123: widget_main->Activate();
On 2015/08/06 01:21:17, tapted wrote:
> On 2015/08/06 00:22:30, jackhou1 wrote:
> > On 2015/08/05 07:07:17, tapted wrote:
> > > I think this should fail since widget->CanActivate returns false, no?
> > 
> > CanActivate() returns false on the other platforms too, it doesn't seem to
> block
> > Activate().
> 
> I think this is a bug though. other platforms should obey CanActivate

I meant that Activate seems to unconditionally activate, and callers are
expected to check CanActivate first. On Mac, Activate calls through to
-makeKeyWindow which checks -canBecomeKeyWindow which calls CanActivate, but on
Ash, CanActivate is called before showing a Widget, or when focus changes.

https://codereview.chromium.org/1268293002/diff/50001/ui/base/test/scoped_fak...
File ui/base/test/scoped_fake_nswindow_focus.mm (right):

https://codereview.chromium.org/1268293002/diff/50001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:65: - (void)makeMainWindow {
On 2015/08/06 01:21:17, tapted wrote:
> just call [self makeKeyWindow]? Then it's clearer that the faker doesn't
really
> try to distinguish them,

Done.

https://codereview.chromium.org/1268293002/diff/50001/ui/base/test/scoped_fak...
ui/base/test/scoped_fake_nswindow_focus.mm:81: new
base::mac::ScopedObjCClassSwizzler([NSWindow class],
On 2015/08/06 01:21:17, tapted wrote:
> a `using base::mac::ScopedObjCClassSwizzler` might convince clang-format to do
a
> tidier job here

Done. Formatting is the same, but a bit easier to read.

[tapted, 2015-08-06 03:01:00]

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:123: widget_main->Activate();
On 2015/08/06 01:51:35, jackhou1 wrote:
> On 2015/08/06 01:21:17, tapted wrote:
> > On 2015/08/06 00:22:30, jackhou1 wrote:
> > > On 2015/08/05 07:07:17, tapted wrote:
> > > > I think this should fail since widget->CanActivate returns false, no?
> > > 
> > > CanActivate() returns false on the other platforms too, it doesn't seem to
> > block
> > > Activate().
> > 
> > I think this is a bug though. other platforms should obey CanActivate
> 
> I meant that Activate seems to unconditionally activate, and callers are
> expected to check CanActivate first. On Mac, Activate calls through to
> -makeKeyWindow which checks -canBecomeKeyWindow which calls CanActivate, but
on
> Ash, CanActivate is called before showing a Widget, or when focus changes.

I think this is only true on ChromeOS (and possibly still a bug). On Windows,
HWNDMessageHandler::OnMouseActivate(..) checks delegate_->CanActivate() and may
return `::MA_NOACTIVATEANDEAT`.

We should get to the bottom of why CanActivate is false. Ideally the faker
should be checking canBecomeKeyWindow (removing the faker and running an
interactive test shouldn't change the behaviour).

It might be something simple like adding 

params_main.activatable = ACTIVATABLE_YES;

before widget_main->Init(..);

But, I'm actually not sure why CanActivate() is false. InitParams::TYPE_POPUP
defaults to can_activate = true in Widget::Init

Maybe the reason is that the Widget is never actually shown?

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:148: // Widget::Deactivate is not
supported on Mac.
I think it's more correct to say that Deactivation can only be properly relied
on in ChromeOS. E.g. on Windows "Deactivate()" is just
find-some-other-window-and-activate-it (if you're the "last" window you can't be
deactivated).

Here, I think it's fine to skip on Mac because set_allow_deactivate_on_esc(true)
is only called from src/ash, but we can perhaps reflect that in the #if (i.e.
!defined(OS_MACOSX) || defined(USE_ASH)

[jackhou1, 2015-08-06 06:10:58]

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:123: widget_main->Activate();
On 2015/08/06 03:01:00, tapted wrote:
> On 2015/08/06 01:51:35, jackhou1 wrote:
> > On 2015/08/06 01:21:17, tapted wrote:
> > > On 2015/08/06 00:22:30, jackhou1 wrote:
> > > > On 2015/08/05 07:07:17, tapted wrote:
> > > > > I think this should fail since widget->CanActivate returns false, no?
> > > > 
> > > > CanActivate() returns false on the other platforms too, it doesn't seem
to
> > > block
> > > > Activate().
> > > 
> > > I think this is a bug though. other platforms should obey CanActivate
> > 
> > I meant that Activate seems to unconditionally activate, and callers are
> > expected to check CanActivate first. On Mac, Activate calls through to
> > -makeKeyWindow which checks -canBecomeKeyWindow which calls CanActivate, but
> on
> > Ash, CanActivate is called before showing a Widget, or when focus changes.
> 
> I think this is only true on ChromeOS (and possibly still a bug). On Windows,
> HWNDMessageHandler::OnMouseActivate(..) checks delegate_->CanActivate() and
may
> return `::MA_NOACTIVATEANDEAT`.

I think the test still works because it only creates Ash widgets.
 
> We should get to the bottom of why CanActivate is false. Ideally the faker
> should be checking canBecomeKeyWindow (removing the faker and running an
> interactive test shouldn't change the behaviour).

Done.
 
> It might be something simple like adding 
> 
> params_main.activatable = ACTIVATABLE_YES;
> 
> before widget_main->Init(..);

Yeah, that works.

> But, I'm actually not sure why CanActivate() is false. InitParams::TYPE_POPUP
> defaults to can_activate = true in Widget::Init

You may have mis-read the logic. It's only activatable if it's *not* TYPE_POPUP
or any of those other types.

> Maybe the reason is that the Widget is never actually shown?

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:148: // Widget::Deactivate is not
supported on Mac.
On 2015/08/06 03:01:00, tapted wrote:
> I think it's more correct to say that Deactivation can only be properly relied
> on in ChromeOS. E.g. on Windows "Deactivate()" is just
> find-some-other-window-and-activate-it (if you're the "last" window you can't
be
> deactivated).
> 
> Here, I think it's fine to skip on Mac because
set_allow_deactivate_on_esc(true)
> is only called from src/ash, but we can perhaps reflect that in the #if (i.e.
> !defined(OS_MACOSX) || defined(USE_ASH)

Done.

[tapted, 2015-08-06 06:49:12]

lgtm

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:123: widget_main->Activate();
On 2015/08/06 06:10:57, jackhou1 wrote:
> On 2015/08/06 03:01:00, tapted wrote:
> > But, I'm actually not sure why CanActivate() is false.
InitParams::TYPE_POPUP
> > defaults to can_activate = true in Widget::Init
> 
> You may have mis-read the logic. It's only activatable if it's *not*
TYPE_POPUP
> or any of those other types.

Ah, so it is. Perhaps comment above the `= ACTIVATABLE_YES` line like, "By
default TYPE_POPUP is not activatable". It should be mentioned in the CL
description too, like

The test was also failing because Mac checks WidgetDelegate::CanActivate() when
activating whereas NativeWidgetAura defers this to `ShouldActivate` which is
only checked by wm::BaseFocusRules

https://codereview.chromium.org/1268293002/diff/110001/ui/views/accessible_pa...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/110001/ui/views/accessible_pa...
ui/views/accessible_pane_view_unittest.cc:147: // Deactivate() is only reliable
on Ash. On Windows it uses ::GetNextWindow
nit: ::GetNextWindow()

https://codereview.chromium.org/1268293002/diff/110001/ui/views/accessible_pa...
ui/views/accessible_pane_view_unittest.cc:148: // simply activates another
window, and which one is not predictable. On Mac,
simply activates -> to simply activate

https://codereview.chromium.org/1268293002/diff/110001/ui/views/accessible_pa...
ui/views/accessible_pane_view_unittest.cc:149: // Deactivate() is not
implemented.
Add:

Note that TestBarView calls set_allow_deactivate_on_esc(true), which is only
otherwise used in Ash.

[jackhou1, 2015-08-06 07:35:03]

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/30001/ui/views/accessible_pan...
ui/views/accessible_pane_view_unittest.cc:123: widget_main->Activate();
On 2015/08/06 06:49:12, tapted wrote:
> On 2015/08/06 06:10:57, jackhou1 wrote:
> > On 2015/08/06 03:01:00, tapted wrote:
> > > But, I'm actually not sure why CanActivate() is false.
> InitParams::TYPE_POPUP
> > > defaults to can_activate = true in Widget::Init
> > 
> > You may have mis-read the logic. It's only activatable if it's *not*
> TYPE_POPUP
> > or any of those other types.
> 
> Ah, so it is. Perhaps comment above the `= ACTIVATABLE_YES` line like, "By
> default TYPE_POPUP is not activatable". It should be mentioned in the CL
> description too, like
> 
> The test was also failing because Mac checks WidgetDelegate::CanActivate()
when
> activating whereas NativeWidgetAura defers this to `ShouldActivate` which is
> only checked by wm::BaseFocusRules

Done.

https://codereview.chromium.org/1268293002/diff/110001/ui/views/accessible_pa...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/110001/ui/views/accessible_pa...
ui/views/accessible_pane_view_unittest.cc:147: // Deactivate() is only reliable
on Ash. On Windows it uses ::GetNextWindow
On 2015/08/06 06:49:12, tapted wrote:
> nit: ::GetNextWindow()

Done.

https://codereview.chromium.org/1268293002/diff/110001/ui/views/accessible_pa...
ui/views/accessible_pane_view_unittest.cc:148: // simply activates another
window, and which one is not predictable. On Mac,
On 2015/08/06 06:49:12, tapted wrote:
> simply activates -> to simply activate

Done.

https://codereview.chromium.org/1268293002/diff/110001/ui/views/accessible_pa...
ui/views/accessible_pane_view_unittest.cc:149: // Deactivate() is not
implemented.
On 2015/08/06 06:49:12, tapted wrote:
> Add:
> 
> Note that TestBarView calls set_allow_deactivate_on_esc(true), which is only
> otherwise used in Ash.

Done.

[jackhou1, 2015-08-06 07:35:39]

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

[jackhou1, 2015-08-06 07:35:40]

sky, please review for OWNERS

[tapted, 2015-08-06 09:07:33]

still lgtm but a rogue symbol appeared

(also yay! this is the last views_unittest \o/)

https://codereview.chromium.org/1268293002/diff/120001/ui/views/accessible_pa...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/120001/ui/views/accessible_pa...
ui/views/accessible_pane_view_unittest.cc:149: // to simply activate∆ another
window, and which one is not predictable. On
nit: a capital "delta" (∆) seems to have appeared in the last patchset

[sky, 2015-08-06 17:10:41]

LGTM

[jackhou1, 2015-08-06 23:40:18]

https://codereview.chromium.org/1268293002/diff/120001/ui/views/accessible_pa...
File ui/views/accessible_pane_view_unittest.cc (right):

https://codereview.chromium.org/1268293002/diff/120001/ui/views/accessible_pa...
ui/views/accessible_pane_view_unittest.cc:149: // to simply activate∆ another
window, and which one is not predictable. On
On 2015/08/06 09:07:33, tapted wrote:
> nit: a capital "delta" (∆) seems to have appeared in the last patchset

Done.

[jackhou1, 2015-08-06 23:40:25]

The CQ bit was checked by jackhou@chromium.org

[jackhou1, 2015-08-06 23:40:26]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1268293002/#ps140001
(title: "Remove roque delta.")

[commit-bot: I haz the power, 2015-08-06 23:40:53]

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

[commit-bot: I haz the power, 2015-08-07 00:44:35]

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

[commit-bot: I haz the power, 2015-08-07 00:44:36]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jackhou1, 2015-08-07 03:20:15]

The CQ bit was checked by jackhou@chromium.org

[commit-bot: I haz the power, 2015-08-07 03:20:41]

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

[commit-bot: I haz the power, 2015-08-07 05:17:31]

Committed patchset #9 (id:140001)

[commit-bot: I haz the power, 2015-08-07 05:18:19]

Patchset 9 (id:??) landed as
https://crrev.com/24869918ffc61b2a29e7a8b23d9120e13c3404fd
Cr-Commit-Position: refs/heads/master@{#342289}