DiFinishNavigation, PageLoaded and URL change events happen when didFailProvisionalNavigation is called. These events should happen after placeholder URL is loaded.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/386114b5bacc02e12b584786671d67b6080f02e7 commit 386114b5bacc02e12b584786671d67b6080f02e7 Author: Eugene But <eugenebut@google.com> Date: Fri Nov 09 16:00:50 2018 Call WebStateObserver::PageLoaded after loading placeholder URL. Before this CL PageLoaded was called right from didFailProvisionalNavigation:, where URL may still represent the previous page. Bug: 903497 Change-Id: I712dd120340353f37d2fe739c5b05e843cb11d7a Reviewed-on: https://chromium-review.googlesource.com/c/1327608 Reviewed-by: Danyao Wang <danyao@chromium.org> Commit-Queue: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#606855} [modify] https://crrev.com/386114b5bacc02e12b584786671d67b6080f02e7/ios/web/web_state/ui/crw_web_controller.mm
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3 commit 7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3 Author: Eugene But <eugenebut@chromium.org> Date: Mon Nov 19 19:38:34 2018 Add web::NavigationContext::IsPlaceholderNavigation This is a step in multistep refactoring. The next steps will be: 1.) web::NavigationContext::URL is never set to placeholder URL and always represents navigation URL 2.) Same web::NavigationContext will be reused for placeholder navigation to extend it's lifetime. 3.) WebStateObserver::DidFinishNavigation will be caller after placeholder navigation is finished and will use original web::NavigationContext passed to WebStateObserver::DidStartNavigation This will partially fix crbug.com/903497 and will call WebStateObserver::DidFinishNavigation after committed URL actually changed. Bug: 903497 Change-Id: Ib6ba96d664ac352d038787aa74ac844d07282789 Reviewed-on: https://chromium-review.googlesource.com/c/1331044 Reviewed-by: Danyao Wang <danyao@chromium.org> Commit-Queue: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#609398} [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/chrome/browser/tabs/BUILD.gn [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/chrome/browser/tabs/DEPS [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/chrome/browser/tabs/tab_unittest.mm [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/web/web_state/BUILD.gn [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/web/web_state/navigation_context_impl.h [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/web/web_state/navigation_context_impl.mm [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/web/web_state/web_state_impl.h [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/web/web_state/web_state_impl.mm [modify] https://crrev.com/7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3/ios/web/web_state/web_state_impl_unittest.mm
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/578c90a6596ae90b476b691867e3e5471090f20b commit 578c90a6596ae90b476b691867e3e5471090f20b Author: Jun Choi <hongjunchoi@chromium.org> Date: Mon Nov 19 23:18:33 2018 Revert "Add web::NavigationContext::IsPlaceholderNavigation" This reverts commit 7b0f2bb852e0280fb3ce78860f7691e4e64ae6f3. Reason for revert: Fails multiple IOS builds including Builder ios-slimnav <https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/ios-slimnav/191> Link to sample failure logs : https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8929398908718170816/+/steps/ios_chrome_unittests__iPhone_6s_iOS_12.1_/0/logs/TextToSpeechListenerTest.ValidAudioDataTest/0 Original change's description: > Add web::NavigationContext::IsPlaceholderNavigation > > This is a step in multistep refactoring. The next steps will be: > > 1.) web::NavigationContext::URL is never set to placeholder > URL and always represents navigation URL > 2.) Same web::NavigationContext will be reused for placeholder > navigation to extend it's lifetime. > 3.) WebStateObserver::DidFinishNavigation will be caller after > placeholder navigation is finished and will use original > web::NavigationContext passed to WebStateObserver::DidStartNavigation > > This will partially fix crbug.com/903497 and will call > WebStateObserver::DidFinishNavigation after committed URL actually > changed. > > Bug: 903497 > Change-Id: Ib6ba96d664ac352d038787aa74ac844d07282789 > Reviewed-on: https://chromium-review.googlesource.com/c/1331044 > Reviewed-by: Danyao Wang <danyao@chromium.org> > Commit-Queue: Eugene But <eugenebut@chromium.org> > Cr-Commit-Position: refs/heads/master@{#609398} TBR=eugenebut@chromium.org,danyao@chromium.org Change-Id: I8d7c9b66d752a792761d95ccf348f045eaa9de03 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 903497 Reviewed-on: https://chromium-review.googlesource.com/c/1343351 Reviewed-by: Jun Choi <hongjunchoi@chromium.org> Commit-Queue: Jun Choi <hongjunchoi@chromium.org> Cr-Commit-Position: refs/heads/master@{#609494} [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/chrome/browser/tabs/BUILD.gn [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/chrome/browser/tabs/DEPS [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/chrome/browser/tabs/tab_unittest.mm [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/web/web_state/BUILD.gn [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/web/web_state/navigation_context_impl.h [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/web/web_state/navigation_context_impl.mm [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/web/web_state/web_state_impl.h [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/web/web_state/web_state_impl.mm [modify] https://crrev.com/578c90a6596ae90b476b691867e3e5471090f20b/ios/web/web_state/web_state_impl_unittest.mm
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a4dede926b598761008de679cb71bc40febbc23 commit 4a4dede926b598761008de679cb71bc40febbc23 Author: Eugene But <eugenebut@chromium.org> Date: Wed Nov 21 22:33:57 2018 Reland Add web::NavigationContext::IsPlaceholderNavigation This is a step in multistep refactoring. The next steps will be: 1.) web::NavigationContext::URL is never set to placeholder URL and always represents navigation URL 2.) Same web::NavigationContext will be reused for placeholder navigation to extend it's lifetime. 3.) WebStateObserver::DidFinishNavigation will be caller after placeholder navigation is finished and will use original web::NavigationContext passed to WebStateObserver::DidStartNavigation This will partially fix crbug.com/903497 and will call WebStateObserver::DidFinishNavigation after committed URL actually changed. Originally Reviewed-on: https://chromium-review.googlesource.com/c/1331044 Bug: 903497 Change-Id: I0ee2ecdfb4d20d2f14fd62e99028e8de1b81ff3b Reviewed-on: https://chromium-review.googlesource.com/c/1347212 Reviewed-by: Danyao Wang <danyao@chromium.org> Commit-Queue: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#610238} [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/chrome/browser/tabs/BUILD.gn [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/chrome/browser/tabs/DEPS [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/chrome/browser/tabs/tab_unittest.mm [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/web/web_state/BUILD.gn [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/web/web_state/navigation_context_impl.h [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/web/web_state/navigation_context_impl.mm [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/web/web_state/web_state_impl.h [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/web/web_state/web_state_impl.mm [modify] https://crrev.com/4a4dede926b598761008de679cb71bc40febbc23/ios/web/web_state/web_state_impl_unittest.mm
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/81321bf5213e465220d5c9515c1786d7b9c9011e commit 81321bf5213e465220d5c9515c1786d7b9c9011e Author: Danyao Wang <danyao@chromium.org> Date: Mon Dec 03 21:47:48 2018 [ios] Remove |loadNativeViewWithSuccess| and inline its content. This makes it more clear which WebStateObserver callbacks are called when. This change does not contain any function changes. Bug: 903497 Change-Id: Ie774c191ffc2681a7cf1df90089ea25e90a381ac Reviewed-on: https://chromium-review.googlesource.com/c/1359296 Reviewed-by: Eugene But <eugenebut@chromium.org> Commit-Queue: Danyao Wang <danyao@chromium.org> Cr-Commit-Position: refs/heads/master@{#613265} [modify] https://crrev.com/81321bf5213e465220d5c9515c1786d7b9c9011e/ios/web/web_state/ui/crw_web_controller.mm
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b129becf000ebcb19d568da0ff5173028f38ee69 commit b129becf000ebcb19d568da0ff5173028f38ee69 Author: Danyao Wang <danyao@chromium.org> Date: Tue Dec 04 15:15:39 2018 [ios] Make |loadCompleteWithSuccess| use web::NavigationContext. This makes it easier in the next step of the refactor to move |loadCompleteWithSuccess| call to |loadErrorPageForNavigationItem|. Bug: 903497 Change-Id: I988c17afee10b56ac9baf720c147a59624afc9f3 Reviewed-on: https://chromium-review.googlesource.com/c/1359320 Reviewed-by: Eugene But <eugenebut@chromium.org> Commit-Queue: Danyao Wang <danyao@chromium.org> Cr-Commit-Position: refs/heads/master@{#613554} [modify] https://crrev.com/b129becf000ebcb19d568da0ff5173028f38ee69/ios/web/web_state/ui/crw_web_controller.mm
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/66d735a8758616bcedf19cefce8554d06de16b50 commit 66d735a8758616bcedf19cefce8554d06de16b50 Author: Danyao Wang <danyao@chromium.org> Date: Tue Dec 04 16:39:11 2018 [ios] Extend the life time of navigation context to error placeholder. Also deferred WebStateObserver::DidFinishNavigation callback to after placeholder load finishes. This removes an inconsistency between legacy nav and slim nav handling of errors. Bug: 903497 Change-Id: I7ee20859b3293da305e2a0ba15d017ea7be0ced8 Reviewed-on: https://chromium-review.googlesource.com/c/1357613 Commit-Queue: Danyao Wang <danyao@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Cr-Commit-Position: refs/heads/master@{#613573} [modify] https://crrev.com/66d735a8758616bcedf19cefce8554d06de16b50/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/66d735a8758616bcedf19cefce8554d06de16b50/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/66d735a8758616bcedf19cefce8554d06de16b50/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/66d735a8758616bcedf19cefce8554d06de16b50/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm [modify] https://crrev.com/66d735a8758616bcedf19cefce8554d06de16b50/ios/web/web_state/web_state_observer_inttest.mm
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d54722d9763c73eba287f436365fc0542d88a2fa commit d54722d9763c73eba287f436365fc0542d88a2fa Author: Robert Flack <flackr@chromium.org> Date: Tue Dec 04 17:54:40 2018 Revert "[ios] Extend the life time of navigation context to error placeholder." This reverts commit 66d735a8758616bcedf19cefce8554d06de16b50. Reason for revert: Suspected cause for the new failures on ios-webview https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/ios-webview/2465 Original change's description: > [ios] Extend the life time of navigation context to error placeholder. > > Also deferred WebStateObserver::DidFinishNavigation callback to after > placeholder load finishes. This removes an inconsistency between legacy > nav and slim nav handling of errors. > > Bug: 903497 > Change-Id: I7ee20859b3293da305e2a0ba15d017ea7be0ced8 > Reviewed-on: https://chromium-review.googlesource.com/c/1357613 > Commit-Queue: Danyao Wang <danyao@chromium.org> > Reviewed-by: Eugene But <eugenebut@chromium.org> > Cr-Commit-Position: refs/heads/master@{#613573} TBR=eugenebut@chromium.org,danyao@chromium.org Change-Id: Ib83d75bca88feb7833f1100c297391caaae6d355 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 903497 Reviewed-on: https://chromium-review.googlesource.com/c/1361640 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: Robert Flack <flackr@chromium.org> Cr-Commit-Position: refs/heads/master@{#613593} [modify] https://crrev.com/d54722d9763c73eba287f436365fc0542d88a2fa/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/d54722d9763c73eba287f436365fc0542d88a2fa/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/d54722d9763c73eba287f436365fc0542d88a2fa/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/d54722d9763c73eba287f436365fc0542d88a2fa/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm [modify] https://crrev.com/d54722d9763c73eba287f436365fc0542d88a2fa/ios/web/web_state/web_state_observer_inttest.mm
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a4f95055278cb4d732b0a7c310bb38cb962070e4 commit a4f95055278cb4d732b0a7c310bb38cb962070e4 Author: Danyao Wang <danyao@chromium.org> Date: Tue Dec 04 23:57:05 2018 Reland "[ios] Extend the life time of navigation context to error placeholder." This is a reland of 66d735a8758616bcedf19cefce8554d06de16b50 after fixing the broken tests. Original change's description: > [ios] Extend the life time of navigation context to error placeholder. > > Also deferred WebStateObserver::DidFinishNavigation callback to after > placeholder load finishes. This removes an inconsistency between legacy > nav and slim nav handling of errors. > > Bug: 903497 > Change-Id: I7ee20859b3293da305e2a0ba15d017ea7be0ced8 > Reviewed-on: https://chromium-review.googlesource.com/c/1357613 > Commit-Queue: Danyao Wang <danyao@chromium.org> > Reviewed-by: Eugene But <eugenebut@chromium.org> > Cr-Commit-Position: refs/heads/master@{#613573} Bug: 903497 Change-Id: I911a5b76c62f9f27f6c32f1981cdb5476c1db10c Reviewed-on: https://chromium-review.googlesource.com/c/1362000 Reviewed-by: Eugene But <eugenebut@chromium.org> Commit-Queue: Danyao Wang <danyao@chromium.org> Cr-Commit-Position: refs/heads/master@{#613768} [modify] https://crrev.com/a4f95055278cb4d732b0a7c310bb38cb962070e4/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/a4f95055278cb4d732b0a7c310bb38cb962070e4/ios/web/web_state/ui/crw_web_controller_unittest.mm [modify] https://crrev.com/a4f95055278cb4d732b0a7c310bb38cb962070e4/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/a4f95055278cb4d732b0a7c310bb38cb962070e4/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/a4f95055278cb4d732b0a7c310bb38cb962070e4/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm [modify] https://crrev.com/a4f95055278cb4d732b0a7c310bb38cb962070e4/ios/web/web_state/web_state_observer_inttest.mm [modify] https://crrev.com/a4f95055278cb4d732b0a7c310bb38cb962070e4/ios/web_view/test/navigation_delegate_inttest.mm
Comment 1 by bugdroid1@chromium.org
, Nov 9