Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2766453002:
Implement Reload for ORIGINAL_REQUEST_URL in NavigationManagerImpl. (Closed)
DescriptionImplement Reload for ORIGINAL_REQUEST_URL in NavigationManagerImpl.
This CL implements logic for |Reload| in NavigationManagerImpl when
reload type is ORIGINAL_REQUEST_URL for reloading a page with updated
user agent.
BUG=700571
Review-Url: https://codereview.chromium.org/2766453002
Cr-Commit-Position: refs/heads/master@{#460161}
Committed: https://chromium.googlesource.com/chromium/src/+/6d1e82b93ec0328ec947f387a84fdf2fd42f61f8
Patch Set 1 : self review #
Total comments: 2
Patch Set 2 : addressed self review #
Total comments: 15
Patch Set 3 : Addressed comments #
Total comments: 21
Patch Set 4 : Addressed comments #
Total comments: 8
Patch Set 5 : Update unit tests #Patch Set 6 : Rebase #
Total comments: 1
Messages
Total messages: 38 (27 generated)
Description was changed from ========== first version BUG= ========== to ========== This CL implements logic for |Reload| in NavigationManagerImpl when reload type is ORIGINAL_REQUEST_URL for reloading a page with updated user agent. This CL also removes the |GetLastNonRedirectedItem| function, which duplicates the logic of ORIGINAL_REQUEST_URL, in NavigationManagerUtil. BUG=700571 ==========
liaoyuke@chromium.org changed reviewers: + eugenebut@chromium.org, kkhorimoto@chromium.org
Description was changed from ========== This CL implements logic for |Reload| in NavigationManagerImpl when reload type is ORIGINAL_REQUEST_URL for reloading a page with updated user agent. This CL also removes the |GetLastNonRedirectedItem| function, which duplicates the logic of ORIGINAL_REQUEST_URL, in NavigationManagerUtil. BUG=700571 ========== to ========== Implement Reload for ORIGINAL_REQUEST_URL in NavigationManagerImpl. This CL implements logic for |Reload| in NavigationManagerImpl when reload type is ORIGINAL_REQUEST_URL for reloading a page with updated user agent. This CL also removes the |GetLastNonRedirectedItem| function, which duplicates the logic of ORIGINAL_REQUEST_URL, in NavigationManagerUtil. BUG=700571 ==========
Patchset #1 (id:1) has been deleted
Patchset #2 (id:40001) has been deleted
Patchset #2 (id:60001) has been deleted
PTAL. Thank you very much! https://codereview.chromium.org/2766453002/diff/20001/ios/web/navigation/navi... File ios/web/navigation/navigation_manager_impl.mm (left): https://codereview.chromium.org/2766453002/diff/20001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:441: DCHECK_GT(index, 0); I don't see any reason why index cannot be 0. Am I missing something?
Description was changed from ========== Implement Reload for ORIGINAL_REQUEST_URL in NavigationManagerImpl. This CL implements logic for |Reload| in NavigationManagerImpl when reload type is ORIGINAL_REQUEST_URL for reloading a page with updated user agent. This CL also removes the |GetLastNonRedirectedItem| function, which duplicates the logic of ORIGINAL_REQUEST_URL, in NavigationManagerUtil. BUG=700571 ========== to ========== Implement Reload for ORIGINAL_REQUEST_URL in NavigationManagerImpl. This CL implements logic for |Reload| in NavigationManagerImpl when reload type is ORIGINAL_REQUEST_URL for reloading a page with updated user agent. BUG=700571 ==========
https://codereview.chromium.org/2766453002/diff/20001/ios/web/navigation/navi... File ios/web/navigation/navigation_manager_impl.mm (left): https://codereview.chromium.org/2766453002/diff/20001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:441: DCHECK_GT(index, 0); On 2017/03/22 23:46:23, liaoyuke wrote: > I don't see any reason why index cannot be 0. Am I missing something? This was a bug :) https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... File ios/web/navigation/navigation_manager_impl.mm (right): https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:346: // For Request Desktop/Mobile Site, we want to reload with the original Avoid "we" in the comments (go/avoidwe) https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:346: // For Request Desktop/Mobile Site, we want to reload with the original This method is written in generic way, independently from Desktop UA feature. So I don't think this method should make assumptions that it is used only for Desktop UA feature. https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:362: DiscardNonCommittedItems(); Is this necessary? Maybe CRWWebController already discards these during the reload? https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:366: while (index >= 0) { Searching for last non-redirect item can be factored in a separate function, but the bigger question is: how this code is different from https://codereview.chromium.org/2756483006/patch/160001/170004? If it's the same, then can we avoid duplicating the code? F.e. would it be simper to fix server redirects code? https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:373: bool item_removed_successfuly = RemoveItemAtIndex(index); Current implementation of Reload does not destroy forward items. https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... File ios/web/navigation/navigation_manager_impl_unittest.mm (right): https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl_unittest.mm:615: false /* check_for_repost */); Can we also verify that NavigationManagerDelegate::Reload() was called? https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl_unittest.mm:758: We should also test reloading with existing forward items
The CQ bit was checked by liaoyuke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by liaoyuke@chromium.org to run a CQ dry run
Patchset #3 (id:100001) has been deleted
Patchset #3 (id:120001) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by liaoyuke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #3 (id:140001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #3 (id:160001) has been deleted
PTAL. Thank you very much! https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... File ios/web/navigation/navigation_manager_impl.mm (right): https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:346: // For Request Desktop/Mobile Site, we want to reload with the original On 2017/03/23 00:08:19, Eugene But wrote: > Avoid "we" in the comments (go/avoidwe) Done. Thank you for providing the link. :) https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:346: // For Request Desktop/Mobile Site, we want to reload with the original On 2017/03/23 00:08:20, Eugene But wrote: > This method is written in generic way, independently from Desktop UA feature. So > I don't think this method should make assumptions that it is used only for > Desktop UA feature. Done. https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:362: DiscardNonCommittedItems(); On 2017/03/23 00:08:19, Eugene But wrote: > Is this necessary? Maybe CRWWebController already discards these during the > reload? Addressed per offline discussion. https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:366: while (index >= 0) { On 2017/03/23 00:08:20, Eugene But wrote: > Searching for last non-redirect item can be factored in a separate function, but > the bigger question is: > how this code is different from > https://codereview.chromium.org/2756483006/patch/160001/170004 > > If it's the same, then can we avoid duplicating the code? F.e. would it be > simper to fix server redirects code? |GetLastNonRedirectedItem()| in NavigationManagerUtil is meant to be replaced by the logic here, that being said, I will delete |GetLastNonRedirectedItem| in a separate CL. btw, |GetLastNonRedirectedItem()| is confusing as it's unclear whether this method counts non-history pending item or not, so I think it's better to not expose it. https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl.mm:373: bool item_removed_successfuly = RemoveItemAtIndex(index); On 2017/03/23 00:08:20, Eugene But wrote: > Current implementation of Reload does not destroy forward items. Addressed per offline discussion. https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... File ios/web/navigation/navigation_manager_impl_unittest.mm (right): https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl_unittest.mm:615: false /* check_for_repost */); On 2017/03/23 00:08:20, Eugene But wrote: > Can we also verify that NavigationManagerDelegate::Reload() was called? Done. https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl_unittest.mm:758: On 2017/03/23 00:08:20, Eugene But wrote: > We should also test reloading with existing forward items Do you mean redirect items exist before last non-redirect item? https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/crw... File ios/web/navigation/crw_session_controller.mm (right): https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/crw... ios/web/navigation/crw_session_controller.mm:547: I see the following code in clearForwardItems: if (_navigationManager) { _navigationManager->OnNavigationItemsPruned(itemCount - forwardItemStartIndex); } So, I guess we wanted to do the same thing here.
https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... File ios/web/navigation/navigation_manager_impl_unittest.mm (right): https://codereview.chromium.org/2766453002/diff/80001/ios/web/navigation/navi... ios/web/navigation/navigation_manager_impl_unittest.mm:758: On 2017/03/27 16:22:43, liaoyuke wrote: > On 2017/03/23 00:08:20, Eugene But wrote: > > We should also test reloading with existing forward items > > Do you mean redirect items exist before last non-redirect item? I mean reloading item which is in the middle of the navigation stack. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/crw... File ios/web/navigation/crw_session_controller.mm (right): https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/crw... ios/web/navigation/crw_session_controller.mm:547: On 2017/03/27 16:22:43, liaoyuke wrote: > I see the following code in clearForwardItems: > > if (_navigationManager) { > _navigationManager->OnNavigationItemsPruned(itemCount - forwardItemStartIndex); > } > > So, I guess we wanted to do the same thing here. Sounds reasonable, but it would be better to make the change in a separate CL. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... File ios/web/navigation/navigation_manager_impl.mm (right): https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:343: // request url of the current item because a server side redirect may change Could you please avoid referring to "current item". Chrome has multiple ways to implement "current item" and this term is quite confusing. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:344: // its url. For example, an user visits www.youtube.com and is then redirected nit: s/www.youtube.com/www.chromium.org and s/m.youtube.com/m.chromium.org https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:353: NavigationItem* current_item = [session_controller_ currentItem]; The end goal is to remove |currentItem|, so it will be better to avoid adding new calls fro this API. According to ReloadType::ORIGINAL_REQUEST_URL this code should reload visible item, so maybe s/currentItem/visibleItem? https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:355: current_item->SetURL(current_item->GetOriginalRequestURL()); Should we crash if NM is empty? Does this code crash for ReloadType::NORMAL reload? https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... File ios/web/navigation/navigation_manager_impl_unittest.mm (right): https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:30: bool ReloadCalled() { return reload_called_; } nit: s/ReloadCalled/reload_called https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:611: // there is no item. nit: s/is no item/are no committed and pending items https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:660: TEST_F(NavigationManagerTest, ReloadUserPendingItemWithNormalType) { Optional nit: Do you want to swap the order for ReloadRendererPendingItemWithOriginalType and ReloadRendererPendingItemWithOriginalType ? https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:698: TEST_F(NavigationManagerTest, ReloadLastCommittedItemWithNormalType) { ditto, do you move this test to the rest of web::ReloadType::NORMAL tests? https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:746: // Tests that calling |Reload| with web::ReloadType::NORMAL leaves the url of ditto
PTAL. Thank you very much! https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/crw... File ios/web/navigation/crw_session_controller.mm (right): https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/crw... ios/web/navigation/crw_session_controller.mm:547: On 2017/03/27 17:15:47, Eugene But wrote: > On 2017/03/27 16:22:43, liaoyuke wrote: > > I see the following code in clearForwardItems: > > > > if (_navigationManager) { > > _navigationManager->OnNavigationItemsPruned(itemCount - > forwardItemStartIndex); > > } > > > > So, I guess we wanted to do the same thing here. > Sounds reasonable, but it would be better to make the change in a separate CL. Acknowledged. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... File ios/web/navigation/navigation_manager_impl.mm (right): https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:343: // request url of the current item because a server side redirect may change On 2017/03/27 17:15:47, Eugene But wrote: > Could you please avoid referring to "current item". Chrome has multiple ways to > implement "current item" and this term is quite confusing. Done. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:344: // its url. For example, an user visits www.youtube.com and is then redirected On 2017/03/27 17:15:47, Eugene But wrote: > nit: http://s/www.youtube.com/www.chromium.org and http://s/m.youtube.com/m.chromium.org Done. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:353: NavigationItem* current_item = [session_controller_ currentItem]; On 2017/03/27 17:15:47, Eugene But wrote: > The end goal is to remove |currentItem|, so it will be better to avoid adding > new calls fro this API. According to ReloadType::ORIGINAL_REQUEST_URL this code > should reload visible item, so maybe s/currentItem/visibleItem? Addressed per offline discussion. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:355: current_item->SetURL(current_item->GetOriginalRequestURL()); On 2017/03/27 17:15:47, Eugene But wrote: > Should we crash if NM is empty? Does this code crash for ReloadType::NORMAL > reload? This function should do nothing if NM is empty. Addressed in a separate CL. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... File ios/web/navigation/navigation_manager_impl_unittest.mm (right): https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:30: bool ReloadCalled() { return reload_called_; } On 2017/03/27 17:15:47, Eugene But wrote: > nit: s/ReloadCalled/reload_called Done. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:611: // there is no item. On 2017/03/27 17:15:47, Eugene But wrote: > nit: s/is no item/are no committed and pending items Done. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:660: TEST_F(NavigationManagerTest, ReloadUserPendingItemWithNormalType) { On 2017/03/27 17:15:47, Eugene But wrote: > Optional nit: Do you want to swap the order for > ReloadRendererPendingItemWithOriginalType and > ReloadRendererPendingItemWithOriginalType ? Done. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:698: TEST_F(NavigationManagerTest, ReloadLastCommittedItemWithNormalType) { On 2017/03/27 17:15:47, Eugene But wrote: > ditto, do you move this test to the rest of web::ReloadType::NORMAL tests? Done. https://codereview.chromium.org/2766453002/diff/180001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:746: // Tests that calling |Reload| with web::ReloadType::NORMAL leaves the url of On 2017/03/27 17:15:47, Eugene But wrote: > ditto Done.
lgtm https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... File ios/web/navigation/navigation_manager_impl.mm (right): https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:346: // For example, an user visits www.chromium.org and is then redirected s/an/the ? https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... File ios/web/navigation/navigation_manager_impl_unittest.mm (right): https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:21: public: How about this? public: bool reload_called() { return reload_called_; } private: // NavigationManagerDelegate overrides void GoToIndex(int index) override {} ... https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:610: // Tests that calling |Reload| with web::ReloadType::NORMAL will not crash when s/will not crash/is no op Same comment for other places https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:618: EXPECT_TRUE(navigation_manager_delegate().reload_called()); Should this be EXPECT_FALSE?
The CQ bit was checked by liaoyuke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thank you very much for reviewing! https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... File ios/web/navigation/navigation_manager_impl.mm (right): https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:346: // For example, an user visits www.chromium.org and is then redirected On 2017/03/28 01:52:56, Eugene But wrote: > s/an/the ? Done. https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... File ios/web/navigation/navigation_manager_impl_unittest.mm (right): https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:21: public: On 2017/03/28 01:52:56, Eugene But wrote: > How about this? > > public: > bool reload_called() { return reload_called_; } > > private: > // NavigationManagerDelegate overrides > void GoToIndex(int index) override {} > ... Done. https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:610: // Tests that calling |Reload| with web::ReloadType::NORMAL will not crash when On 2017/03/28 01:52:56, Eugene But wrote: > s/will not crash/is no op > > Same comment for other places Done. https://codereview.chromium.org/2766453002/diff/200001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl_unittest.mm:618: EXPECT_TRUE(navigation_manager_delegate().reload_called()); On 2017/03/28 01:52:56, Eugene But wrote: > Should this be EXPECT_FALSE? Done.
The CQ bit was checked by liaoyuke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from eugenebut@chromium.org Link to the patchset: https://codereview.chromium.org/2766453002/#ps240001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 240001, "attempt_start_ts": 1490721074406210,
"parent_rev": "94a0a47011c54d22e7e64e0f7f9d62ce9e9e82d0", "commit_rev":
"6d1e82b93ec0328ec947f387a84fdf2fd42f61f8"}
Message was sent while issue was closed.
Description was changed from ========== Implement Reload for ORIGINAL_REQUEST_URL in NavigationManagerImpl. This CL implements logic for |Reload| in NavigationManagerImpl when reload type is ORIGINAL_REQUEST_URL for reloading a page with updated user agent. BUG=700571 ========== to ========== Implement Reload for ORIGINAL_REQUEST_URL in NavigationManagerImpl. This CL implements logic for |Reload| in NavigationManagerImpl when reload type is ORIGINAL_REQUEST_URL for reloading a page with updated user agent. BUG=700571 Review-Url: https://codereview.chromium.org/2766453002 Cr-Commit-Position: refs/heads/master@{#460161} Committed: https://chromium.googlesource.com/chromium/src/+/6d1e82b93ec0328ec947f387a84f... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:240001) as https://chromium.googlesource.com/chromium/src/+/6d1e82b93ec0328ec947f387a84f...
Message was sent while issue was closed.
https://codereview.chromium.org/2766453002/diff/240001/ios/web/navigation/nav... File ios/web/navigation/navigation_manager_impl.mm (right): https://codereview.chromium.org/2766453002/diff/240001/ios/web/navigation/nav... ios/web/navigation/navigation_manager_impl.mm:360: reload_item = GetLastCommittedItem(); So are we no longer using the last user item logic? What's the explanation for this change in behavior?
Message was sent while issue was closed.
I will update the bug right now to explain. On Tue, Mar 28, 2017 at 11:10 AM <kkhorimoto@chromium.org> wrote: > > > https://codereview.chromium.org/2766453002/diff/240001/ios/web/navigation/nav... > File ios/web/navigation/navigation_manager_impl.mm (right): > > > https://codereview.chromium.org/2766453002/diff/240001/ios/web/navigation/nav... > ios/web/navigation/navigation_manager_impl.mm:360: reload_item = > GetLastCommittedItem(); > So are we no longer using the last user item logic? What's the > explanation for this change in behavior? > > https://codereview.chromium.org/2766453002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
