Commit 9e64858d authored by Justin Cohen's avatar Justin Cohen Committed by Commit Bot

[ios] Correct non-crwnative NTP title.

NTP title should be set earlier in DidStartNavigation.  It was previously moved
to didFinish to sync up with changes to ios/web.

Also adds an OWNERS file.

Bug: 826369
Change-Id: I1f65118aa16425b44b96e14802638e67f1acd24d
Reviewed-on: https://chromium-review.googlesource.com/c/1308495
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604340}
parent 3398fa51
...@@ -27,6 +27,7 @@ source_set("unit_tests") { ...@@ -27,6 +27,7 @@ source_set("unit_tests") {
] ]
deps = [ deps = [
"//base/test:test_support", "//base/test:test_support",
"//components/strings:components_strings_grit",
"//ios/chrome/browser", "//ios/chrome/browser",
"//ios/chrome/browser/browser_state:test_support", "//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/favicon:favicon", "//ios/chrome/browser/favicon:favicon",
...@@ -41,5 +42,6 @@ source_set("unit_tests") { ...@@ -41,5 +42,6 @@ source_set("unit_tests") {
"//ios/web/public/test/fakes", "//ios/web/public/test/fakes",
"//testing/gtest:gtest", "//testing/gtest:gtest",
"//third_party/ocmock", "//third_party/ocmock",
"//ui/base:base",
] ]
} }
...@@ -37,12 +37,17 @@ class NewTabPageTabHelper : public web::WebStateObserver, ...@@ -37,12 +37,17 @@ class NewTabPageTabHelper : public web::WebStateObserver,
// web::WebStateObserver overrides: // web::WebStateObserver overrides:
void WebStateDestroyed(web::WebState* web_state) override; void WebStateDestroyed(web::WebState* web_state) override;
void DidStartNavigation(web::WebState* web_state,
web::NavigationContext* navigation_context) override;
void DidFinishNavigation(web::WebState* web_state, void DidFinishNavigation(web::WebState* web_state,
web::NavigationContext* navigation_context) override; web::NavigationContext* navigation_context) override;
// Enable or disable the tab helper. // Enable or disable the tab helper.
void SetActive(bool active); void SetActive(bool active);
// Sets the NTP's NavigationItem title to the appropriate string.
void UpdatePendingItemTitle();
// Used to present and dismiss the NTP. // Used to present and dismiss the NTP.
__weak id<NewTabPageTabHelperDelegate> delegate_ = nil; __weak id<NewTabPageTabHelperDelegate> delegate_ = nil;
......
...@@ -47,8 +47,10 @@ NewTabPageTabHelper::NewTabPageTabHelper( ...@@ -47,8 +47,10 @@ NewTabPageTabHelper::NewTabPageTabHelper(
web_state->AddObserver(this); web_state->AddObserver(this);
active_ = web_state->GetVisibleURL().GetOrigin() == kChromeUINewTabURL; active_ = web_state->GetVisibleURL().GetOrigin() == kChromeUINewTabURL;
if (active_) if (active_) {
UpdatePendingItemTitle();
[delegate_ newTabPageHelperDidChangeVisibility:this forWebState:web_state_]; [delegate_ newTabPageHelperDidChangeVisibility:this forWebState:web_state_];
}
} }
bool NewTabPageTabHelper::IsActive() const { bool NewTabPageTabHelper::IsActive() const {
...@@ -66,6 +68,14 @@ void NewTabPageTabHelper::WebStateDestroyed(web::WebState* web_state) { ...@@ -66,6 +68,14 @@ void NewTabPageTabHelper::WebStateDestroyed(web::WebState* web_state) {
SetActive(false); SetActive(false);
} }
void NewTabPageTabHelper::DidStartNavigation(
web::WebState* web_state,
web::NavigationContext* navigation_context) {
if (navigation_context->GetUrl().GetOrigin() == kChromeUINewTabURL) {
UpdatePendingItemTitle();
}
}
void NewTabPageTabHelper::DidFinishNavigation( void NewTabPageTabHelper::DidFinishNavigation(
web::WebState* web_state, web::WebState* web_state,
web::NavigationContext* navigation_context) { web::NavigationContext* navigation_context) {
...@@ -82,15 +92,15 @@ void NewTabPageTabHelper::SetActive(bool active) { ...@@ -82,15 +92,15 @@ void NewTabPageTabHelper::SetActive(bool active) {
bool was_active = active_; bool was_active = active_;
active_ = active; active_ = active;
if (active) {
web::NavigationManager* manager = web_state_->GetNavigationManager();
web::NavigationItem* item = manager->GetPendingItem();
if (item)
item->SetTitle(l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE));
}
// Tell |delegate_| to show or hide the NTP, if necessary. // Tell |delegate_| to show or hide the NTP, if necessary.
if (active_ != was_active) { if (active_ != was_active) {
[delegate_ newTabPageHelperDidChangeVisibility:this forWebState:web_state_]; [delegate_ newTabPageHelperDidChangeVisibility:this forWebState:web_state_];
} }
} }
void NewTabPageTabHelper::UpdatePendingItemTitle() {
web::NavigationManager* manager = web_state_->GetNavigationManager();
web::NavigationItem* item = manager->GetPendingItem();
if (item)
item->SetTitle(l10n_util::GetStringUTF16(IDS_NEW_TAB_TITLE));
}
...@@ -7,8 +7,10 @@ ...@@ -7,8 +7,10 @@
#include <memory> #include <memory>
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/strings/sys_string_conversions.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "components/strings/grit/components_strings.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state_manager.h" #include "ios/chrome/browser/browser_state/test_chrome_browser_state_manager.h"
#include "ios/chrome/browser/chrome_url_constants.h" #include "ios/chrome/browser/chrome_url_constants.h"
...@@ -25,8 +27,10 @@ ...@@ -25,8 +27,10 @@
#import "ios/web/public/test/fakes/test_web_state.h" #import "ios/web/public/test/fakes/test_web_state.h"
#include "ios/web/public/test/test_web_thread_bundle.h" #include "ios/web/public/test/test_web_thread_bundle.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "testing/gtest_mac.h"
#include "testing/platform_test.h" #include "testing/platform_test.h"
#import "third_party/ocmock/OCMock/OCMock.h" #import "third_party/ocmock/OCMock/OCMock.h"
#include "ui/base/l10n/l10n_util.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support." #error "This file requires ARC support."
...@@ -94,6 +98,8 @@ TEST_F(NewTabPageTabHelperTest, TestAlreadyNTP) { ...@@ -94,6 +98,8 @@ TEST_F(NewTabPageTabHelperTest, TestAlreadyNTP) {
test_web_state_.SetVisibleURL(url); test_web_state_.SetVisibleURL(url);
CreateTabHelper(); CreateTabHelper();
EXPECT_TRUE(tab_helper()->IsActive()); EXPECT_TRUE(tab_helper()->IsActive());
EXPECT_NSEQ(l10n_util::GetNSString(IDS_NEW_TAB_TITLE),
base::SysUTF16ToNSString(pending_item_->GetTitle()));
} }
// Tests a newly created non-NTP webstate. // Tests a newly created non-NTP webstate.
...@@ -105,6 +111,7 @@ TEST_F(NewTabPageTabHelperTest, TestNotNTP) { ...@@ -105,6 +111,7 @@ TEST_F(NewTabPageTabHelperTest, TestNotNTP) {
test_web_state_.SetVisibleURL(url); test_web_state_.SetVisibleURL(url);
CreateTabHelper(); CreateTabHelper();
EXPECT_FALSE(tab_helper()->IsActive()); EXPECT_FALSE(tab_helper()->IsActive());
EXPECT_NSEQ(@"", base::SysUTF16ToNSString(pending_item_->GetTitle()));
} }
// Tests navigating back and forth between an NTP and non-NTP page. // Tests navigating back and forth between an NTP and non-NTP page.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment