Documentation states that GetPendingItemIndex should return -1 on new navigation.
https://cs.chromium.org/chromium/src/ios/web/public/navigation_manager.h?q=navigationma&sq=package:chromium&l=129

But it returns the current index
https://cs.chromium.org/chromium/src/ios/web/navigation/navigation_manager_impl.mm?sq=package:chromium&l=265

This leads to have

GetPendingItem() != GetItemAtIndex(GetPendingItemIndex()).

This force to add workaround for Reading list offline loading.

Thanks Olivier! GetPendingItemIndex() is only used in IOSLiveTab and IOSChromeSyncedTabDelegate, but I don't know what will happen if we fix GetPendingItemIndex() to return correct value :(.

I'll investigate this one.

I dug into this a bit today because it'll allow me to move GetVisibleItem() implementation from CRWSessionController to NavigationManagerImpl.

Based on the history of NavigationManagerImpl and CRWSessionController, it looks like returning current index may be a result of NavigationManagerImpl::GetPendingEntryIndex() predating existence of pendingEntryIndex concept in CRWSessionManager.

I don't know how to look up downstream history so I don't know why GetPendingEntryIndex() was introduced. But since it predated pendingEntryIndex in CRWSessionController, returning currentNavigationIndex may have been the most feasible option at the time.

Would running through ios-simulator-eg@ tests be sufficient to sanity check the effect of fixing GetPendingItemIndex()?

I think we have to audit every call of GetPendingItemIndex() to understand if the change is safe. WFIW, I looked into one call place and |GetPendingEntryIndex| call was not even useful (https://codereview.chromium.org/2762363002/patch/1/10001).

Running ios-simulator-eg@ will not guarantee the change safety, because our EG test suite does not have many navigation tests.