Inline call to Tab -originalTitle.

There was only one call to Tab -originalTitle, so inline it and
remove the property from Tab.

Bug: none
Change-Id: Idc574da6818df54eac30df23afb636444b119456
Reviewed-on: https://chromium-review.googlesource.com/720170
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509290}

ios/chrome/browser/tabs/tab.h

@@ -96,9 +96,6 @@ extern NSString* const kProxyPassthroughHeaderValue;
 // The current title of the tab.
 @property(nonatomic, readonly) NSString* title;
 
-// Original page title or nil if the page did not provide one.
-@property(nonatomic, readonly) NSString* originalTitle;
-
 @property(nonatomic, readonly) NSString* urlDisplayString;
 
 // ID associated with this tab.

ios/chrome/browser/tabs/tab.mm

@@ -395,17 +395,6 @@ void TabInfoBarObserver::OnInfoBarReplaced(infobars::InfoBar* old_infobar,
   return base::SysUTF16ToNSString(title);
 }
 
-- (NSString*)originalTitle {
-  // Do not use self.webState->GetTitle() as it returns the display title,
-  // not the original page title.
-  DCHECK([self navigationManager]);
-  web::NavigationItem* item = [self navigationManager]->GetLastCommittedItem();
-  if (!item)
-    return nil;
-  base::string16 pageTitle = item->GetTitle();
-  return pageTitle.empty() ? nil : base::SysUTF16ToNSString(pageTitle);
-}
-
 - (NSString*)urlDisplayString {
   base::string16 urlText = url_formatter::FormatUrl(
       self.webState->GetVisibleURL(), url_formatter::kFormatUrlOmitNothing,

ios/chrome/browser/ui/activity_services/share_to_data_builder.mm

@@ -5,9 +5,12 @@
 #include "ios/chrome/browser/ui/activity_services/share_to_data_builder.h"
 
 #include "base/logging.h"
+#import "base/strings/sys_string_conversions.h"
 #include "ios/chrome/browser/tabs/tab.h"
 #include "ios/chrome/browser/ui/activity_services/chrome_activity_item_thumbnail_generator.h"
 #include "ios/chrome/browser/ui/activity_services/share_to_data.h"
+#import "ios/web/public/navigation_item.h"
+#import "ios/web/public/navigation_manager.h"
 #import "ios/web/public/web_state/web_state.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -17,6 +20,7 @@
 namespace activity_services {
 
 ShareToData* ShareToDataForTab(Tab* tab) {
+  DCHECK(tab);
   // For crash documented in crbug.com/503955, tab.url which is being passed
   // as a reference parameter caused a crash due to invalid address which
   // which suggests that |tab| may be deallocated along the way. Check that
@@ -24,19 +28,33 @@ ShareToData* ShareToDataForTab(Tab* tab) {
   // tab is being closed.
   if (!tab.webState)
     return nil;
-  DCHECK(tab);
-  // If the original page title exists, it is expected to match the tab title.
-  // If this ever changes, then a decision has to be made on which one should
-  // be used for sharing.
-  DCHECK(!tab.originalTitle || [tab.originalTitle isEqualToString:tab.title]);
-  BOOL isPagePrintable = [tab viewForPrinting] != nil;
-  ThumbnailGeneratorBlock thumbnailGenerator =
+
+  BOOL is_original_title = NO;
+  DCHECK(tab.webState->GetNavigationManager());
+  web::NavigationItem* last_committed_item =
+      tab.webState->GetNavigationManager()->GetLastCommittedItem();
+  if (last_committed_item) {
+    // Do not use WebState::GetTitle() as it returns the display title, not the
+    // original page title.
+    const base::string16& original_title = last_committed_item->GetTitle();
+    if (!original_title.empty()) {
+      // If the original page title exists, it is expected to match the Tab's
+      // title. If this ever changes, then a decision has to be made on which
+      // one should be used for sharing.
+      DCHECK(
+          [tab.title isEqualToString:base::SysUTF16ToNSString(original_title)]);
+      is_original_title = YES;
+    }
+  }
+
+  BOOL is_page_printable = [tab viewForPrinting] != nil;
+  ThumbnailGeneratorBlock thumbnail_generator =
       activity_services::ThumbnailGeneratorForTab(tab);
   return [[ShareToData alloc] initWithURL:tab.webState->GetVisibleURL()
                                     title:tab.title
-                          isOriginalTitle:(tab.originalTitle != nil)
-                          isPagePrintable:isPagePrintable
-                       thumbnailGenerator:thumbnailGenerator];
+                          isOriginalTitle:is_original_title
+                          isPagePrintable:is_page_printable
+                       thumbnailGenerator:thumbnail_generator];
 }
 
 }  // namespace activity_services

ios/chrome/browser/ui/activity_services/share_to_data_builder_unittest.mm

@@ -6,10 +6,14 @@
 
 #include <memory>
 
+#include "base/macros.h"
+#include "base/strings/sys_string_conversions.h"
+#include "base/strings/utf_string_conversions.h"
 #import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
 #import "ios/chrome/browser/tabs/tab.h"
 #import "ios/chrome/browser/ui/activity_services/share_to_data.h"
 #import "ios/testing/ocmock_complex_type_helper.h"
+#import "ios/web/public/test/fakes/test_navigation_manager.h"
 #import "ios/web/public/test/fakes/test_web_state.h"
 #include "ios/web/public/test/test_web_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -22,6 +26,11 @@
 #error "This file requires ARC support."
 #endif
 
+namespace {
+const char kExpectedUrl[] = "http://www.testurl.net/";
+const char kExpectedTitle[] = "title";
+}  // namespace
+
 @interface ShareToDataBuilderTestTabMock : OCMockComplexTypeHelper {
   std::unique_ptr<web::TestWebState> _webState;
 }
@@ -45,45 +54,70 @@
   return self;
 }
 
+- (void)close {
+  _webState.reset();
+}
+
 @end
 
-using ShareToDataBuilderTest = PlatformTest;
+class ShareToDataBuilderTest : public PlatformTest {
+ public:
+  ShareToDataBuilderTest() {
+    chrome_browser_state_ = TestChromeBrowserState::Builder().Build();
 
-// Verifies that ShareToData is constructed properly for a given Tab.
-TEST_F(ShareToDataBuilderTest, TestSharePageCommandHandling) {
-  GURL expected_url("http://www.testurl.net");
-  NSString* expected_title = @"title";
-
-  web::TestWebThreadBundle thread_bundle;
-  TestChromeBrowserState::Builder test_cbs_builder;
-  std::unique_ptr<ios::ChromeBrowserState> chrome_browser_state =
-      test_cbs_builder.Build();
+    auto navigation_manager = std::make_unique<web::TestNavigationManager>();
+    navigation_manager->AddItem(GURL(kExpectedUrl), ui::PAGE_TRANSITION_TYPED);
+    navigation_manager->SetLastCommittedItem(navigation_manager->GetItemAtIndex(
+        navigation_manager->GetLastCommittedItemIndex()));
+    navigation_manager->GetLastCommittedItem()->SetTitle(
+        base::UTF8ToUTF16(kExpectedTitle));
 
     auto web_state = std::make_unique<web::TestWebState>();
-  web_state->SetBrowserState(chrome_browser_state.get());
-  web_state->SetVisibleURL(expected_url);
+    web_state->SetNavigationManager(std::move(navigation_manager));
+    web_state->SetBrowserState(chrome_browser_state_.get());
+    web_state->SetVisibleURL(GURL(kExpectedUrl));
 
-  ShareToDataBuilderTestTabMock* tab = [[ShareToDataBuilderTestTabMock alloc]
+    tab_ = [[ShareToDataBuilderTestTabMock alloc]
         initWithWebState:std::move(web_state)];
+    OCMockObject* tab_mock = static_cast<OCMockObject*>(tab_);
 
-  OCMockObject* tab_mock = static_cast<OCMockObject*>(tab);
-
-  ios::ChromeBrowserState* ptr = chrome_browser_state.get();
+    ios::ChromeBrowserState* ptr = chrome_browser_state_.get();
+    NSString* expected_title = base::SysUTF8ToNSString(kExpectedTitle);
     [[[tab_mock stub] andReturnValue:OCMOCK_VALUE(ptr)] browserState];
     [[[tab_mock stub] andReturn:expected_title] title];
-  [[[tab_mock stub] andReturn:expected_title] originalTitle];
 
-  UIImage* tab_snapshot = ui::test::uiimage_utils::UIImageWithSizeAndSolidColor(
+    UIImage* tab_snapshot =
+        ui::test::uiimage_utils::UIImageWithSizeAndSolidColor(
             CGSizeMake(300, 400), [UIColor blueColor]);
     [[[tab_mock stub] andReturn:tab_snapshot] generateSnapshotWithOverlay:NO
                                                          visibleFrameOnly:YES];
+  }
 
-  ShareToData* actual_data =
-      activity_services::ShareToDataForTab(static_cast<Tab*>(tab));
+  void TearDown() override {
+    [tab_ close];
+    tab_ = nil;
+    PlatformTest::TearDown();
+  }
+
+  Tab* tab() { return static_cast<Tab*>(tab_); }
+
+  ShareToDataBuilderTestTabMock* tab_mock() { return tab_; }
+
+ private:
+  web::TestWebThreadBundle thread_bundle_;
+  std::unique_ptr<ios::ChromeBrowserState> chrome_browser_state_;
+  ShareToDataBuilderTestTabMock* tab_;
+
+  DISALLOW_COPY_AND_ASSIGN(ShareToDataBuilderTest);
+};
+
+// Verifies that ShareToData is constructed properly for a given Tab.
+TEST_F(ShareToDataBuilderTest, TestSharePageCommandHandling) {
+  ShareToData* actual_data = activity_services::ShareToDataForTab(tab());
 
   ASSERT_TRUE(actual_data);
-  EXPECT_EQ(expected_url, actual_data.url);
-  EXPECT_NSEQ(expected_title, actual_data.title);
+  EXPECT_EQ(kExpectedUrl, actual_data.url);
+  EXPECT_NSEQ(base::SysUTF8ToNSString(kExpectedTitle), actual_data.title);
   EXPECT_TRUE(actual_data.isOriginalTitle);
   EXPECT_FALSE(actual_data.isPagePrintable);
 
@@ -97,9 +131,7 @@ TEST_F(ShareToDataBuilderTest, TestSharePageCommandHandling) {
 // Verifies that |ShareToDataForTab()| returns nil if the Tab is in the process
 // of being closed.
 TEST_F(ShareToDataBuilderTest, TestReturnsNilWhenClosing) {
-  // Sets WebState to nullptr because [tab close] clears the WebState.
-  ShareToDataBuilderTestTabMock* tab =
-      [[ShareToDataBuilderTestTabMock alloc] initWithWebState:nullptr];
+  [tab_mock() close];
 
-  EXPECT_EQ(nil, activity_services::ShareToDataForTab(static_cast<Tab*>(tab)));
+  EXPECT_EQ(nil, activity_services::ShareToDataForTab(tab()));
 }