diff --git a/chrome/VERSION b/chrome/VERSION index 4e03bd3..53a292e2 100644 --- a/chrome/VERSION +++ b/chrome/VERSION
@@ -1,4 +1,4 @@ MAJOR=58 MINOR=0 -BUILD=3010 +BUILD=3011 PATCH=0
diff --git a/ios/chrome/browser/tabs/tab.mm b/ios/chrome/browser/tabs/tab.mm index 9c5817c..7dd1d6e 100644 --- a/ios/chrome/browser/tabs/tab.mm +++ b/ios/chrome/browser/tabs/tab.mm
@@ -337,10 +337,6 @@ // Sets the favicon on the current NavigationItem. - (void)setFavicon:(const gfx::Image*)image; -// Updates the title field of the current session entry. Also updates the -// history database. -- (void)updateTitle:(NSString*)title; - // Saves the current title to the history database. - (void)saveTitleToHistoryDB; @@ -993,20 +989,6 @@ overscrollActionsControllerDelegate); } -- (void)updateTitle:(NSString*)title { - web::NavigationItem* item = [self navigationManager]->GetVisibleItem(); - if (!item) - return; - item->SetTitle(base::SysNSStringToUTF16(title)); - // TODO(crbug.com/546218): See if this can be removed; it's not clear that - // other platforms send this (tab sync triggers need to be compared against - // upstream). - if (webStateImpl_) - webStateImpl_->GetNavigationManagerImpl().OnNavigationItemChanged(); - - [self saveTitleToHistoryDB]; -} - - (void)saveTitleToHistoryDB { // If incognito, don't update history. if (browserState_->IsOffTheRecord()) @@ -1831,13 +1813,8 @@ - (void)webController:(CRWWebController*)webController titleDidChange:(NSString*)title { - NSString* oldTitle = [self title]; - BOOL isTitleChanged = (!oldTitle && title) || (oldTitle && !title) || - (![oldTitle isEqualToString:title]); - if (isTitleChanged) { - [self updateTitle:title]; - [parentTabModel_ notifyTabChanged:self]; - } + [self saveTitleToHistoryDB]; + [parentTabModel_ notifyTabChanged:self]; } - (BOOL)urlTriggersNativeAppLaunch:(const GURL&)url
diff --git a/ios/chrome/browser/tabs/tab_unittest.mm b/ios/chrome/browser/tabs/tab_unittest.mm index c611f47d..d863fa3 100644 --- a/ios/chrome/browser/tabs/tab_unittest.mm +++ b/ios/chrome/browser/tabs/tab_unittest.mm
@@ -242,6 +242,8 @@ [[tab_ navigationManager]->GetSessionController() commitPendingItem]; [[tab_ webController] webStateImpl]->OnNavigationCommitted(redirectUrl); [tab_ webDidStartLoadingURL:redirectUrl shouldUpdateHistory:YES]; + base::string16 new_title = base::SysNSStringToUTF16(title); + [tab_ navigationManager]->GetLastCommittedItem()->SetTitle(new_title); [tab_ webController:mock_web_controller_ titleDidChange:title]; [[[(id)mock_web_controller_ expect] andReturnValue:OCMOCK_VALUE(kPageLoaded)] loadPhase];
diff --git a/ios/web/web_state/ui/crw_web_controller.mm b/ios/web/web_state/ui/crw_web_controller.mm index 8eacd96..19a95a6 100644 --- a/ios/web/web_state/ui/crw_web_controller.mm +++ b/ios/web/web_state/ui/crw_web_controller.mm
@@ -702,6 +702,9 @@ stateObject:(NSString*)stateObject; // Sets _documentURL to newURL, and updates any relevant state information. - (void)setDocumentURL:(const GURL&)newURL; +// Sets last committed NavigationItem's title to the given |title|, which can +// not be nil. +- (void)setNavigationItemTitle:(NSString*)title; // Returns YES if the current navigation item corresponds to a web page // loaded by a POST request. - (BOOL)isCurrentNavigationItemPOST; @@ -1368,6 +1371,28 @@ } } +- (void)setNavigationItemTitle:(NSString*)title { + DCHECK(title); + auto& navigationManager = _webStateImpl->GetNavigationManagerImpl(); + web::NavigationItem* item = navigationManager.GetLastCommittedItem(); + if (!item) + return; + + base::string16 newTitle = base::SysNSStringToUTF16(title); + if (item->GetTitle() == newTitle) + return; + + item->SetTitle(newTitle); + // TODO(crbug.com/546218): See if this can be removed; it's not clear that + // other platforms send this (tab sync triggers need to be compared against + // upstream). + navigationManager.OnNavigationItemChanged(); + + if ([_delegate respondsToSelector:@selector(webController:titleDidChange:)]) { + [_delegate webController:self titleDidChange:title]; + } +} + - (BOOL)isCurrentNavigationItemPOST { // |_pendingNavigationInfo| will be nil if the decidePolicy* delegate methods // were not called. @@ -1792,14 +1817,13 @@ // Perform post-load-finished updates. [self didFinishWithURL:currentURL loadSuccess:loadSuccess]; - // Inform the embedder the title changed. + NSString* title = [self.nativeController title]; + if (title) + [self setNavigationItemTitle:title]; + + // If the controller handles title change notification, route those to the + // delegate. if ([_delegate respondsToSelector:@selector(webController:titleDidChange:)]) { - NSString* title = [self.nativeController title]; - // If a title is present, notify the delegate. - if (title) - [_delegate webController:self titleDidChange:title]; - // If the controller handles title change notification, route those to the - // delegate. if ([self.nativeController respondsToSelector:@selector(setDelegate:)]) { [self.nativeController setDelegate:self]; } @@ -4774,6 +4798,10 @@ // Attempt to update the HTML5 history state. [self updateHTML5HistoryState]; + // This is the point where pending entry has been committed, and navigation + // item title should be updated. + [self setNavigationItemTitle:[_webView title]]; + // Report cases where SSL cert is missing for a secure connection. if (_documentURL.SchemeIsCryptographic()) { scoped_refptr<net::X509Certificate> cert = @@ -4969,10 +4997,12 @@ return; } - if ([self.delegate - respondsToSelector:@selector(webController:titleDidChange:)]) { - DCHECK([_webView title]); - [self.delegate webController:self titleDidChange:[_webView title]]; + bool hasPendingNavigation = web::WKNavigationState::COMMITTED <= + [_navigationStates lastAddedNavigationState]; + if (hasPendingNavigation) { + // Do not update the title if there is a navigation in progress because + // there is no way to tell if KVO change fired for new or previous page. + [self setNavigationItemTitle:[_webView title]]; } }
diff --git a/ios/web/web_state/ui/crw_web_controller_unittest.mm b/ios/web/web_state/ui/crw_web_controller_unittest.mm index 75bc292..44638df 100644 --- a/ios/web/web_state/ui/crw_web_controller_unittest.mm +++ b/ios/web/web_state/ui/crw_web_controller_unittest.mm
@@ -778,6 +778,7 @@ TEST_F(CRWWebControllerTest, WebUrlWithTrustLevel) { [[[mockWebView_ stub] andReturn:[NSURL URLWithString:@(kTestURLString)]] URL]; [[[mockWebView_ stub] andReturnBool:NO] hasOnlySecureContent]; + [[[mockWebView_ stub] andReturn:@""] title]; // Stub out the injection process. [[mockWebView_ stub] evaluateJavaScript:OCMOCK_ANY
diff --git a/ios/web/web_state/ui/crw_wk_navigation_states.h b/ios/web/web_state/ui/crw_wk_navigation_states.h index 5005e40..c16fc5b 100644 --- a/ios/web/web_state/ui/crw_wk_navigation_states.h +++ b/ios/web/web_state/ui/crw_wk_navigation_states.h
@@ -54,6 +54,10 @@ // last added navigation. Returns nil if there are no stored navigations. - (WKNavigation*)lastAddedNavigation; +// State of WKNavigation which was added the most recently via +// |setState:forNavigation:|. +- (web::WKNavigationState)lastAddedNavigationState; + @end #endif // IOS_WEB_WEB_STATE_UI_CRW_WK_NAVIGATION_STATES_H_
diff --git a/ios/web/web_state/ui/crw_wk_navigation_states.mm b/ios/web/web_state/ui/crw_wk_navigation_states.mm index f15ebc09..59f1c6d 100644 --- a/ios/web/web_state/ui/crw_wk_navigation_states.mm +++ b/ios/web/web_state/ui/crw_wk_navigation_states.mm
@@ -105,4 +105,13 @@ return result; } +- (web::WKNavigationState)lastAddedNavigationState { + CRWWKNavigationsStateRecord* lastAddedRecord = nil; + WKNavigation* lastAddedNavigation = [self lastAddedNavigation]; + if (lastAddedNavigation) + lastAddedRecord = [_records objectForKey:lastAddedNavigation]; + + return lastAddedRecord.state; +} + @end
diff --git a/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm b/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm index 61c4409..5b3eb4f3 100644 --- a/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm +++ b/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm
@@ -31,14 +31,22 @@ // navigation_1 is the only navigation and it is the latest. [states_ setState:WKNavigationState::REQUESTED forNavigation:navigation1_]; EXPECT_EQ(navigation1_, [states_ lastAddedNavigation]); + EXPECT_EQ(WKNavigationState::REQUESTED, [states_ lastAddedNavigationState]); // navigation_2 is added later and hence the latest. [states_ setState:WKNavigationState::REQUESTED forNavigation:navigation2_]; EXPECT_EQ(navigation2_, [states_ lastAddedNavigation]); + EXPECT_EQ(WKNavigationState::REQUESTED, [states_ lastAddedNavigationState]); // Updating state for existing navigation does not make it the latest. [states_ setState:WKNavigationState::STARTED forNavigation:navigation1_]; EXPECT_EQ(navigation2_, [states_ lastAddedNavigation]); + EXPECT_EQ(WKNavigationState::REQUESTED, [states_ lastAddedNavigationState]); + + // navigation_2 is still the latest. + [states_ setState:WKNavigationState::STARTED forNavigation:navigation2_]; + EXPECT_EQ(navigation2_, [states_ lastAddedNavigation]); + EXPECT_EQ(WKNavigationState::STARTED, [states_ lastAddedNavigationState]); } } // namespace web