Use correct position when setting pos_in_set for tabs.

chrome/browser/ui/views/tabs/tab_strip.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <iterator>
@@ skipping 1326 matching lines @@
       IDR_THEME_TAB_BACKGROUND_INCOGNITO : IDR_THEME_TAB_BACKGROUND;
   *custom_image =
       tp->HasCustomImage(id) || tp->HasCustomImage(IDR_THEME_FRAME) ||
       (incognito && tp->HasCustomImage(IDR_THEME_FRAME_INCOGNITO));
   return id;
 }
 
 void TabStrip::UpdateTabAccessibilityState(const Tab* tab,
                                            ui::AXNodeData* node_data) {
   node_data->AddIntAttribute(ui::AX_ATTR_SET_SIZE, tab_count());
-  node_data->AddIntAttribute(ui::AX_ATTR_POS_IN_SET, GetModelIndexOfTab(tab));
+
+  int pos_in_set = tab_count() ? GetModelIndexOfTab(tab) + 1 : 0;

[sky, 2017/02/28 04:01:05]

GetModelIndexOfTab may return -1 if tab is closing.

[David Tseng, 2017/02/28 19:26:52]

Added explicit check (which omits data if it is not satisfied).

+  node_data->AddIntAttribute(ui::AX_ATTR_POS_IN_SET, pos_in_set);

[sky, 2017/02/28 04:02:33]

One more comment. Why do we need AX_ATTR_POS_IN_SET to be 1s based? Can't we
hide that in however the data is exported? Forcing everyone to deal with 1s
based is so easy to get wrong (as we just proved).

[David Tseng, 2017/02/28 19:26:52]

It's spec'ed as 1-based in the relevant W3C doc (ARIA).  It's handled that way
in Blink, for example.

The -1 case is actually deeper than accessibility. The Tab becomes visible with
a tab index of -1, so I'm not really sure if I understand when a view should be
capable of providing accurate information about its state or if such a thing
exists or should exist.

I've added a note in the ax_enums.idl file and added an explicit check for the
expected values.

It's a pretty obscure attribute in the first place and I don't think your
general audience here would know to use it. The original change was made by
Dominic in the context of a larger refactor.

 }
 
 void TabStrip::MouseMovedOutOfHost() {
   ResizeLayoutTabs();
   if (reset_to_shrink_on_exit_) {
     reset_to_shrink_on_exit_ = false;
     SetStackedLayout(false);
     controller_->StackedLayoutMaybeChanged();
   }
 }
@@ skipping 1423 matching lines @@
           ConvertPointToViewAndGetEventHandler(this, newtab_button_, point);
       if (view)
         return view;
     }
     Tab* tab = FindTabForEvent(point);
     if (tab)
       return ConvertPointToViewAndGetEventHandler(this, tab, point);
   }
   return this;
 }