Skip to content

Commit c0b7fe9

Browse files
Eugene ButKariah Davis
authored andcommitted
Gracefully handle null pending item inside webView:didCommitNavigation:.
webView:didCommitNavigation: used to assume that pending item is always valid and crashed on dereferencing null pointer. Pending item should never be null inside webView:didCommitNavigation: but existing ownership model is incorrect, so pending item could be distroyed before committing. This CL adds checks before dereferencing pending item to avoid the crash. The real fix could be implemented by storing pending item in NavigationContext object (crbug.com/925304). Bug: 676458 Change-Id: Idf60e60cbe98111e8fed5d2903ae8bc8b8df3d90 Reviewed-on: https://chromium-review.googlesource.com/c/1444953 Reviewed-by: Justin Cohen <[email protected]> Commit-Queue: Eugene But <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#627509}(cherry picked from commit 010e2b2) Reviewed-on: https://chromium-review.googlesource.com/c/1448521 Reviewed-by: Kariah Davis <[email protected]> Cr-Commit-Position: refs/branch-heads/3683@{#94} Cr-Branched-From: e510299-refs/heads/master@{#625896}
1 parent 2a145bf commit c0b7fe9

File tree

1 file changed

+25
-10
lines changed

1 file changed

+25
-10
lines changed

ios/web/web_state/ui/crw_web_controller.mm

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,6 +1718,12 @@ - (void)updateCurrentBackForwardListItemHolder {
17181718
if (_webUIManager)
17191719
return;
17201720

1721+
if (!self.currentNavItem) {
1722+
// TODO(crbug.com/925304): Pending item (which stores the holder) should be
1723+
// owned by NavigationContext object. Pending item should never be null.
1724+
return;
1725+
}
1726+
17211727
web::WKBackForwardListItemHolder* holder =
17221728
[self currentBackForwardListItemHolder];
17231729

@@ -2933,7 +2939,10 @@ - (void)webPageChangedWithContext:(const web::NavigationContextImpl*)context {
29332939
// keep it since it will have a more accurate URL and policy than what can
29342940
// be extracted from the landing page.)
29352941
web::NavigationItem* currentItem = self.currentNavItem;
2936-
if (!currentItem->GetReferrer().url.is_valid()) {
2942+
2943+
// TODO(crbug.com/925304): Pending item (which should be used here) should be
2944+
// owned by NavigationContext object. Pending item should never be null.
2945+
if (currentItem && !currentItem->GetReferrer().url.is_valid()) {
29372946
currentItem->SetReferrer(referrer);
29382947
}
29392948

@@ -4940,15 +4949,21 @@ - (void)webView:(WKWebView*)webView
49404949
}
49414950

49424951
[self commitPendingNavigationInfo];
4943-
if ([self currentBackForwardListItemHolder]->navigation_type() ==
4944-
WKNavigationTypeBackForward) {
4945-
// A fast back/forward won't call decidePolicyForNavigationResponse, so
4946-
// the MIME type needs to be updated explicitly.
4947-
NSString* storedMIMEType =
4948-
[self currentBackForwardListItemHolder]->mime_type();
4949-
if (storedMIMEType) {
4950-
self.webStateImpl->SetContentsMimeType(
4951-
base::SysNSStringToUTF8(storedMIMEType));
4952+
4953+
// TODO(crbug.com/925304): Pending item (which is stores
4954+
// currentBackForwardListItemHolder) should be owned by NavigationContext.
4955+
// Pending item should never be null.
4956+
if (self.currentNavItem) {
4957+
if ([self currentBackForwardListItemHolder]
4958+
-> navigation_type() == WKNavigationTypeBackForward) {
4959+
// A fast back/forward won't call decidePolicyForNavigationResponse, so
4960+
// the MIME type needs to be updated explicitly.
4961+
NSString* storedMIMEType =
4962+
[self currentBackForwardListItemHolder] -> mime_type();
4963+
if (storedMIMEType) {
4964+
self.webStateImpl->SetContentsMimeType(
4965+
base::SysNSStringToUTF8(storedMIMEType));
4966+
}
49524967
}
49534968
}
49544969

0 commit comments

Comments
 (0)