Revert my recent changes regarding title directionality.

This reverts: r82400: Plumb direction of document title through IPC layer. r82582: Add and use a base::i18n::StringWithDirection for carrying titles. r82778: Change NavigationEntry's title fields to carry the text direction. I'm going to take an alternative approach; after getting this far, I can see that this approach was too complicated. BUG=27094 Review URL: http://codereview.chromium.org/6901003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@82908 0039d316-1c4b-4281-b951-d872f2087c98
Denger-Network · Apr 25, 2011 · 6b2f7a8 · 6b2f7a8
1 parent 434d5c0
commit 6b2f7a8
Show file tree

Hide file tree

Showing 33 changed files with 116 additions and 239 deletions.
diff --git a/base/i18n/rtl.h b/base/i18n/rtl.h
@@ -31,28 +31,6 @@ enum TextDirection {
   LEFT_TO_RIGHT,
 };
 
-// A string along with the text direction it should be displayed in.
-// Conceptually this is a struct; we just use 'class' to make it easier for
-// others to forward-declare us with 'class String16WithDirection'.
-class String16WithDirection {
- public:
-  String16WithDirection() : direction_(UNKNOWN_DIRECTION) { }
-  String16WithDirection(const string16& str, TextDirection dir)
-      : string_(str), direction_(dir) { }
-
-  const string16& string() const { return string_; }
-  TextDirection direction() const { return direction_; }
-
-  bool is_empty() const { return string_.empty(); }
-  bool operator==(const String16WithDirection& other) const {
-    return string_ == other.string_ && direction_ == other.direction_;
-  }
-
- private:
-  string16 string_;
-  TextDirection direction_;
-};
-
 // Get the locale that the currently running process has been configured to use.
 // The return value is of the form language[-country] (e.g., en-US) where the
 // language is the 2 or 3 letter code from ISO-639.

diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
@@ -1025,9 +1025,7 @@ void TestingAutomationProvider::GetTabTitle(int handle,
     NavigationController* tab = tab_tracker_->GetResource(handle);
     NavigationEntry* entry = tab->GetActiveEntry();
     if (entry != NULL) {
-      // TODO(evan): use directionality of title.
-      // http://code.google.com/p/chromium/issues/detail?id=27094
-      *title = UTF16ToWideHack(entry->GetTitleForDisplay("").string());
+      *title = UTF16ToWideHack(entry->GetTitleForDisplay(""));
     } else {
       *title = std::wstring();
     }

diff --git a/chrome/browser/debugger/devtools_http_protocol_handler.cc b/chrome/browser/debugger/devtools_http_protocol_handler.cc
@@ -253,9 +253,7 @@ static PageList GeneratePageList(
     page_info.id = controller.session_id().id();
     page_info.attached = client_host != NULL;
     page_info.url = entry->url().spec();
-    // TODO(evan): use directionality of title?
-    // http://code.google.com/p/chromium/issues/detail?id=27094
-    page_info.title = UTF16ToUTF8(entry->title().string());
+    page_info.title = UTF16ToUTF8(entry->title());
     page_info.favicon_url = entry->favicon().url().spec();
     page_list.push_back(page_info);
   }

diff --git a/chrome/browser/external_tab_container_win.cc b/chrome/browser/external_tab_container_win.cc
@@ -884,9 +884,7 @@ bool ExternalTabContainer::InitNavigationInfo(NavigationInfo* nav_info,
       tab_contents_->controller().GetCurrentEntryIndex();
   nav_info->url = entry->url();
   nav_info->referrer = entry->referrer();
-  // TODO(evan): use directionality of title.
-  // http://code.google.com/p/chromium/issues/detail?id=27094
-  nav_info->title = UTF16ToWideHack(entry->title().string());
+  nav_info->title =  UTF16ToWideHack(entry->title());
   if (nav_info->title.empty())
     nav_info->title = UTF8ToWide(nav_info->url.spec());
 

diff --git a/chrome/browser/notifications/balloon_host.h b/chrome/browser/notifications/balloon_host.h
@@ -56,10 +56,8 @@ class BalloonHost : public RenderViewHostDelegate,
   virtual void RenderViewGone(RenderViewHost* render_view_host,
                               base::TerminationStatus status,
                               int error_code);
-  virtual void UpdateTitle(
-      RenderViewHost* render_view_host,
-      int32 page_id,
-      const base::i18n::String16WithDirection& title) OVERRIDE {}
+  virtual void UpdateTitle(RenderViewHost* render_view_host,
+                           int32 page_id, const std::wstring& title) {}
   virtual int GetBrowserWindowID() const;
   virtual ViewType::Type GetRenderViewType() const;
   virtual RenderViewHostDelegate::View* GetViewDelegate();

diff --git a/chrome/browser/prerender/prerender_contents.cc b/chrome/browser/prerender/prerender_contents.cc
@@ -4,7 +4,6 @@
 
 #include "chrome/browser/prerender/prerender_contents.h"
 
-#include "base/i18n/rtl.h"
 #include "base/process_util.h"
 #include "base/task.h"
 #include "base/utf_string_conversions.h"
@@ -277,17 +276,14 @@ void PrerenderContents::DidNavigate(
   url_ = params.url;
 }
 
-void PrerenderContents::UpdateTitle(
-    RenderViewHost* render_view_host,
-    int32 page_id,
-    const base::i18n::String16WithDirection& title) {
+void PrerenderContents::UpdateTitle(RenderViewHost* render_view_host,
+                                    int32 page_id,
+                                    const std::wstring& title) {
   DCHECK_EQ(render_view_host_, render_view_host);
-  if (title.string().empty())
+  if (title.empty())
     return;
 
-  // TODO(evan): use directionality of title.
-  // http://code.google.com/p/chromium/issues/detail?id=27094
-  title_ = title.string();
+  title_ = WideToUTF16Hack(title);
   page_id_ = page_id;
 }
 

diff --git a/chrome/browser/prerender/prerender_contents.h b/chrome/browser/prerender/prerender_contents.h
@@ -120,10 +120,9 @@ class PrerenderContents : public RenderViewHostDelegate,
                               int error_code) OVERRIDE;
   virtual void DidNavigate(RenderViewHost* render_view_host,
                            const ViewHostMsg_FrameNavigate_Params& params);
-  virtual void UpdateTitle(
-      RenderViewHost* render_view_host,
-      int32 page_id,
-      const base::i18n::String16WithDirection& title) OVERRIDE;
+  virtual void UpdateTitle(RenderViewHost* render_view_host,
+                           int32 page_id,
+                           const std::wstring& title);
   virtual WebPreferences GetWebkitPrefs();
   virtual void RunJavaScriptMessage(const std::wstring& message,
                                     const std::wstring& default_prompt,

diff --git a/chrome/browser/prerender/prerender_manager.cc b/chrome/browser/prerender/prerender_manager.cc
@@ -376,14 +376,9 @@ bool PrerenderManager::MaybeUsePreloadedPage(TabContents* tc, const GURL& url) {
   if (p != NULL)
     tc->DidNavigate(rvh, *p);
 
-  // TODO(evan): use directionality of title.
-  // http://code.google.com/p/chromium/issues/detail?id=27094
-  string16 title_str = pc->title();
-  if (!title_str.empty()) {
-    base::i18n::String16WithDirection title(title_str,
-                                            base::i18n::LEFT_TO_RIGHT);
-    tc->UpdateTitle(rvh, pc->page_id(), title);
-  }
+  string16 title = pc->title();
+  if (!title.empty())
+    tc->UpdateTitle(rvh, pc->page_id(), UTF16ToWideHack(title));
 
   GURL icon_url = pc->icon_url();
   if (!icon_url.is_empty()) {

diff --git a/chrome/browser/sessions/base_session_service.cc b/chrome/browser/sessions/base_session_service.cc
@@ -158,10 +158,7 @@ SessionCommand* BaseSessionService::CreateUpdateTabNavigationCommand(
   WriteStringToPickle(pickle, &bytes_written, max_state_size,
                       entry.virtual_url().spec());
 
-  // TODO(evan): use directionality of title.
-  // http://code.google.com/p/chromium/issues/detail?id=27094
-  WriteString16ToPickle(pickle, &bytes_written, max_state_size,
-                        entry.title().string());
+  WriteString16ToPickle(pickle, &bytes_written, max_state_size, entry.title());
 
   if (entry.has_post_data()) {
     // Remove the form data, it may contain sensitive information.

diff --git a/chrome/browser/sessions/session_service_unittest.cc b/chrome/browser/sessions/session_service_unittest.cc
@@ -71,11 +71,7 @@ class SessionServiceTest : public BrowserWithTestWindowTest,
     NavigationEntry entry;
     entry.set_url(navigation.virtual_url());
     entry.set_referrer(navigation.referrer());
-    // TODO(evan): use directionality of title.
-    // http://code.google.com/p/chromium/issues/detail?id=27094
-    entry.set_title(
-        base::i18n::String16WithDirection(navigation.title(),
-                                          base::i18n::LEFT_TO_RIGHT));
+    entry.set_title(navigation.title());
     entry.set_content_state(navigation.state());
     entry.set_transition_type(navigation.transition());
     entry.set_has_post_data(

diff --git a/chrome/browser/sessions/session_types.cc b/chrome/browser/sessions/session_types.cc
@@ -68,10 +68,7 @@ NavigationEntry* TabNavigation::ToNavigationEntry(int page_id,
       profile);
 
   entry->set_page_id(page_id);
-  // TODO(evan): use directionality of title.
-  // http://code.google.com/p/chromium/issues/detail?id=27094
-  entry->set_title(
-      base::i18n::String16WithDirection(title_, base::i18n::LEFT_TO_RIGHT));
+  entry->set_title(title_);
   entry->set_content_state(state_);
   entry->set_has_post_data(type_mask_ & TabNavigation::HAS_POST_DATA);
 
@@ -81,9 +78,7 @@ NavigationEntry* TabNavigation::ToNavigationEntry(int page_id,
 void TabNavigation::SetFromNavigationEntry(const NavigationEntry& entry) {
   virtual_url_ = entry.virtual_url();
   referrer_ = entry.referrer();
-  // TODO(evan): use directionality of title.
-  // http://code.google.com/p/chromium/issues/detail?id=27094
-  title_ = entry.title().string();
+  title_ = entry.title();
   state_ = entry.content_state();
   transition_ = entry.transition_type();
   type_mask_ = entry.has_post_data() ? TabNavigation::HAS_POST_DATA : 0;

diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc
@@ -237,10 +237,7 @@ TEST_F(TabContentsTest, UpdateTitle) {
   NavigationController::LoadCommittedDetails details;
   controller().RendererDidNavigate(params, 0, &details);
 
-  base::i18n::String16WithDirection new_title(
-      ASCIIToUTF16("    Lots O' Whitespace\n"),
-      base::i18n::LEFT_TO_RIGHT);
-  contents()->UpdateTitle(rvh(), 0, new_title);
+  contents()->UpdateTitle(rvh(), 0, L"    Lots O' Whitespace\n");
   EXPECT_EQ(ASCIIToUTF16("Lots O' Whitespace"), contents()->GetTitle());
 }
 

diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
@@ -4426,7 +4426,7 @@ void Browser::ViewSource(TabContentsWrapper* contents,
       webkit_glue::RemoveScrollOffsetFromHistoryState(content_state));
 
   // Do not restore title, derive it from the url.
-  active_entry->set_title(base::i18n::String16WithDirection());
+  active_entry->set_title(string16());
 
   // Now show view-source entry.
   if (CanSupportWindowFeature(FEATURE_TABSTRIP)) {

diff --git a/chrome/browser/ui/cocoa/applescript/tab_applescript.mm b/chrome/browser/ui/cocoa/applescript/tab_applescript.mm
@@ -125,10 +125,9 @@ - (NSString*)title {
     return nil;
 
   std::wstring title;
-  // TODO(evan): use directionality of title.
-  // http://code.google.com/p/chromium/issues/detail?id=27094
-  if (entry != NULL)
-    title = UTF16ToWideHack(entry->title().string());
+  if (entry != NULL) {
+    title = UTF16ToWideHack(entry->title());
+  }
 
   return base::SysWideToNSString(title);
 }

diff --git a/chrome/browser/ui/toolbar/back_forward_menu_model.cc b/chrome/browser/ui/toolbar/back_forward_menu_model.cc
@@ -84,22 +84,19 @@ string16 BackForwardMenuModel::GetLabelAt(int index) const {
   // Return the entry title, escaping any '&' characters and eliding it if it's
   // super long.
   NavigationEntry* entry = GetNavigationEntry(index);
-  base::i18n::String16WithDirection menu_text(entry->GetTitleForDisplay(
+  string16 menu_text(entry->GetTitleForDisplay(
       GetTabContents()->profile()->GetPrefs()->
           GetString(prefs::kAcceptLanguages)));
-  string16 elided =
-      ui::ElideText(menu_text.string(), gfx::Font(), kMaxWidth, false);
+  menu_text = ui::ElideText(menu_text, gfx::Font(), kMaxWidth, false);
 
 #if !defined(OS_MACOSX)
-  for (size_t i = elided.find('&'); i != string16::npos;
-       i = elided.find('&', i + 2)) {
-    elided.insert(i, 1, '&');
+  for (size_t i = menu_text.find('&'); i != string16::npos;
+       i = menu_text.find('&', i + 2)) {
+    menu_text.insert(i, 1, '&');
   }
 #endif
 
-  // TODO(evan): use directionality of title.
-  // http://code.google.com/p/chromium/issues/detail?id=27094
-  return elided;
+  return menu_text;
 }
 
 bool BackForwardMenuModel::IsItemDynamicAt(int index) const {

diff --git a/chrome/browser/ui/toolbar/back_forward_menu_model_unittest.cc b/chrome/browser/ui/toolbar/back_forward_menu_model_unittest.cc
@@ -73,9 +73,7 @@ class BackFwdMenuModelTest : public RenderViewHostTestHarness {
 
   void LoadURLAndUpdateState(const char* url, const char* title) {
     NavigateAndCommit(GURL(url));
-    controller().GetLastCommittedEntry()->set_title(
-        base::i18n::String16WithDirection(UTF8ToUTF16(title),
-                                          base::i18n::LEFT_TO_RIGHT));
+    controller().GetLastCommittedEntry()->set_title(UTF8ToUTF16(title));
   }
 
   // Navigate back or forward the given amount and commits the entry (which

diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
@@ -399,9 +399,7 @@ bool GetCurrentTabTitle(const Browser* browser, string16* title) {
   NavigationEntry* last_entry = tab_contents->controller().GetActiveEntry();
   if (!last_entry)
     return false;
-  // TODO(evan): use directionality of title.
-  // http://code.google.com/p/chromium/issues/detail?id=27094
-  title->assign(last_entry->GetTitleForDisplay("").string());
+  title->assign(last_entry->GetTitleForDisplay(""));
   return true;
 }
 

diff --git a/content/browser/renderer_host/render_view_host.cc b/content/browser/renderer_host/render_view_host.cc
@@ -999,20 +999,13 @@ void RenderViewHost::OnMsgUpdateState(int32 page_id,
   delegate_->UpdateState(this, page_id, state);
 }
 
-void RenderViewHost::OnMsgUpdateTitle(
-    int32 page_id,
-    const string16& title,
-    WebKit::WebTextDirection title_direction) {
+void RenderViewHost::OnMsgUpdateTitle(int32 page_id,
+                                      const std::wstring& title) {
   if (title.length() > content::kMaxTitleChars) {
     NOTREACHED() << "Renderer sent too many characters in title.";
     return;
   }
-  base::i18n::TextDirection dir =
-      title_direction == WebKit::WebTextDirectionLeftToRight ?
-          base::i18n::LEFT_TO_RIGHT :
-          base::i18n::RIGHT_TO_LEFT;
-  delegate_->UpdateTitle(this, page_id,
-                         base::i18n::String16WithDirection(title, dir));
+  delegate_->UpdateTitle(this, page_id, title);
 }
 
 void RenderViewHost::OnMsgUpdateEncoding(const std::string& encoding_name) {

diff --git a/content/browser/renderer_host/render_view_host.h b/content/browser/renderer_host/render_view_host.h
@@ -528,8 +528,7 @@ class RenderViewHost : public RenderWidgetHost {
   void OnMsgNavigate(const IPC::Message& msg);
   void OnMsgUpdateState(int32 page_id,
                         const std::string& state);
-  void OnMsgUpdateTitle(int32 page_id, const string16& title,
-                        WebKit::WebTextDirection title_direction);
+  void OnMsgUpdateTitle(int32 page_id, const std::wstring& title);
   void OnMsgUpdateEncoding(const std::string& encoding);
   void OnMsgUpdateTargetURL(int32 page_id, const GURL& url);
   void OnMsgScreenshot(const SkBitmap& bitmap);

diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h
@@ -55,9 +55,6 @@ class WebKeyboardEvent;
 struct WebPreferences;
 
 namespace base {
-namespace i18n {
-class String16WithDirection;
-}
 class WaitableEvent;
 }
 
@@ -439,7 +436,7 @@ class RenderViewHostDelegate : public IPC::Channel::Listener {
   // The page's title was changed and should be updated.
   virtual void UpdateTitle(RenderViewHost* render_view_host,
                            int32 page_id,
-                           const base::i18n::String16WithDirection& title) {}
+                           const std::wstring& title) {}
 
   // The page's encoding was changed and should be updated.
   virtual void UpdateEncoding(RenderViewHost* render_view_host,

diff --git a/content/browser/site_instance_unittest.cc b/content/browser/site_instance_unittest.cc
@@ -137,16 +137,16 @@ TEST_F(SiteInstanceTest, SiteInstanceDestructor) {
   EXPECT_EQ(0, siteDeleteCounter);
 
   NavigationEntry* e1 = new NavigationEntry(instance, 0, url, GURL(),
-                                            base::i18n::String16WithDirection(),
+                                            string16(),
                                             PageTransition::LINK);
 
   // Redundantly setting e1's SiteInstance shouldn't affect the ref count.
   e1->set_site_instance(instance);
   EXPECT_EQ(0, siteDeleteCounter);
 
   // Add a second reference
-  NavigationEntry* e2 = new NavigationEntry(instance, 0, url, GURL(),
-                                            base::i18n::String16WithDirection(),
+  NavigationEntry* e2 = new NavigationEntry(instance, 0, url,
+                                            GURL(), string16(),
                                             PageTransition::LINK);
 
   // Now delete both entries and be sure the SiteInstance goes away.
@@ -197,7 +197,7 @@ TEST_F(SiteInstanceTest, CloneNavigationEntry) {
                                                &browsingDeleteCounter);
 
   NavigationEntry* e1 = new NavigationEntry(instance1, 0, url, GURL(),
-                                            base::i18n::String16WithDirection(),
+                                            string16(),
                                             PageTransition::LINK);
   // Clone the entry
   NavigationEntry* e2 = new NavigationEntry(*e1);

diff --git a/content/browser/tab_contents/interstitial_page.cc b/content/browser/tab_contents/interstitial_page.cc
@@ -270,7 +270,7 @@ void InterstitialPage::Hide() {
   // Let's revert to the original title if necessary.
   NavigationEntry* entry = tab_->controller().GetActiveEntry();
   if (!new_navigation_ && should_revert_tab_title_) {
-    entry->set_title(original_tab_title_);
+    entry->set_title(WideToUTF16Hack(original_tab_title_));
     tab_->NotifyNavigationStateChanged(TabContents::INVALIDATE_TITLE);
   }
   delete this;
@@ -383,10 +383,9 @@ void InterstitialPage::DidNavigate(
   tab_->SetIsLoading(false, NULL);
 }
 
-void InterstitialPage::UpdateTitle(
-    RenderViewHost* render_view_host,
-    int32 page_id,
-    const base::i18n::String16WithDirection& title) {
+void InterstitialPage::UpdateTitle(RenderViewHost* render_view_host,
+                                   int32 page_id,
+                                   const std::wstring& title) {
   DCHECK(render_view_host == render_view_host_);
   NavigationEntry* entry = tab_->controller().GetActiveEntry();
   if (!entry) {
@@ -405,10 +404,10 @@ void InterstitialPage::UpdateTitle(
   // If this interstitial is shown on an existing navigation entry, we'll need
   // to remember its title so we can revert to it when hidden.
   if (!new_navigation_ && !should_revert_tab_title_) {
-    original_tab_title_ = entry->title();
+    original_tab_title_ = UTF16ToWideHack(entry->title());
     should_revert_tab_title_ = true;
   }
-  entry->set_title(title);
+  entry->set_title(WideToUTF16Hack(title));
   tab_->NotifyNavigationStateChanged(TabContents::INVALIDATE_TITLE);
 }