From c5b0633bdd495bb1ea78ed50b1b74520de444ab2 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Fri, 8 Feb 2013 16:42:22 +0000 Subject: [PATCH] Send OnTitleChange() notifications when navigating history (issue #766). git-svn-id: https://chromiumembedded.googlecode.com/svn/trunk@1087 5089003a-bbd8-11dd-ad1f-f1f9622dbc98 --- cef.gyp | 1 + libcef/browser/browser_host_impl.cc | 49 +++++++++---- libcef/browser/browser_host_impl.h | 3 + tests/unittests/display_unittest.cc | 104 ++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 14 deletions(-) create mode 100644 tests/unittests/display_unittest.cc diff --git a/cef.gyp b/cef.gyp index 36f9f3a29..58d0a84a7 100644 --- a/cef.gyp +++ b/cef.gyp @@ -245,6 +245,7 @@ 'tests/unittests/command_line_unittest.cc', 'tests/unittests/cookie_unittest.cc', 'tests/unittests/dialog_unittest.cc', + 'tests/unittests/display_unittest.cc', 'tests/unittests/dom_unittest.cc', 'tests/unittests/geolocation_unittest.cc', 'tests/unittests/jsdialog_unittest.cc', diff --git a/libcef/browser/browser_host_impl.cc b/libcef/browser/browser_host_impl.cc index 7bb4fd2e5..e523ffd43 100644 --- a/libcef/browser/browser_host_impl.cc +++ b/libcef/browser/browser_host_impl.cc @@ -1855,22 +1855,34 @@ void CefBrowserHostImpl::OnResponseAck(int request_id) { void CefBrowserHostImpl::Observe(int type, const content::NotificationSource& source, const content::NotificationDetails& details) { - DCHECK(type == content::NOTIFICATION_WEB_CONTENTS_TITLE_UPDATED || - type == content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE); + DCHECK(type == content::NOTIFICATION_LOAD_STOP || + type == content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE || + type == content::NOTIFICATION_WEB_CONTENTS_TITLE_UPDATED); - if (type == content::NOTIFICATION_WEB_CONTENTS_TITLE_UPDATED) { - std::pair* title = - content::Details >( - details).ptr(); + if (type == content::NOTIFICATION_LOAD_STOP || + type == content::NOTIFICATION_WEB_CONTENTS_TITLE_UPDATED) { + string16 title; - if (title->first) { - if (client_.get()) { - CefRefPtr handler = client_->GetDisplayHandler(); - if (handler.get()) { - CefString title_str = title->first->GetTitleForDisplay(""); - handler->OnTitleChange(this, title_str); - } - } + if (type == content::NOTIFICATION_LOAD_STOP) { + content::NavigationController* controller = + content::Source(source).ptr(); + title = controller->GetWebContents()->GetTitle(); + } else { + content::WebContents* web_contents = + content::Source(source).ptr(); + title = web_contents->GetTitle(); + } + + // Don't notify if the title hasn't changed. + if (title == title_) + return; + + title_ = title; + + if (client_.get()) { + CefRefPtr handler = client_->GetDisplayHandler(); + if (handler.get()) + handler->OnTitleChange(this, title); } } else if (type == content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE) { focus_on_editable_field_ = *content::Details(details).ptr(); @@ -1914,6 +1926,15 @@ CefBrowserHostImpl::CefBrowserHostImpl( registrar_->Add(this, content::NOTIFICATION_WEB_CONTENTS_TITLE_UPDATED, content::Source(web_contents)); + // When navigating through the history, the restored NavigationEntry's title + // will be used. If the entry ends up having the same title after we return + // to it, as will usually be the case, the + // NOTIFICATION_WEB_CONTENTS_TITLE_UPDATED will then be suppressed, since the + // NavigationEntry's title hasn't changed. + registrar_->Add(this, content::NOTIFICATION_LOAD_STOP, + content::Source( + &web_contents->GetController())); + response_manager_.reset(new CefResponseManager); placeholder_frame_ = diff --git a/libcef/browser/browser_host_impl.h b/libcef/browser/browser_host_impl.h index 025b8283e..0a90f7b14 100644 --- a/libcef/browser/browser_host_impl.h +++ b/libcef/browser/browser_host_impl.h @@ -499,6 +499,9 @@ class CefBrowserHostImpl : public CefBrowserHost, // True if a file chooser is currently pending. bool file_chooser_pending_; + // Current title for the main frame. Only accessed on the UI thread. + string16 title_; + IMPLEMENT_REFCOUNTING(CefBrowserHostImpl); DISALLOW_EVIL_CONSTRUCTORS(CefBrowserHostImpl); }; diff --git a/tests/unittests/display_unittest.cc b/tests/unittests/display_unittest.cc new file mode 100644 index 000000000..9149e29ee --- /dev/null +++ b/tests/unittests/display_unittest.cc @@ -0,0 +1,104 @@ +// Copyright (c) 2013 The Chromium Embedded Framework Authors. All rights +// reserved. Use of this source code is governed by a BSD-style license that +// can be found in the LICENSE file. + +#include +#include "include/cef_runnable.h" +#include "tests/unittests/test_handler.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// How it works: +// 1. Load kTitleUrl1 (title should be kTitleStr1) +// 2. Load kTitleUrl2 (title should be kTitleStr2) +// 3. History back to kTitleUrl1 (title should be kTitleStr1) +// 4. History forward to kTitleUrl2 (title should be kTitleStr2) +// 5. Set title via JavaScript (title should be kTitleStr3) + +const char kTitleUrl1[] = "http://tests-title/nav1.html"; +const char kTitleUrl2[] = "http://tests-title/nav2.html"; +const char kTitleStr1[] = "Title 1"; +const char kTitleStr2[] = "Title 2"; +const char kTitleStr3[] = "Title 3"; + +// Browser side. +class TitleTestHandler : public TestHandler { + public: + TitleTestHandler() + : step_(0) {} + + virtual void RunTest() OVERRIDE { + // Add the resources that we will navigate to/from. + AddResource(kTitleUrl1, + "" + std::string(kTitleStr1) + + "Nav1", "text/html"); + AddResource(kTitleUrl2, + "" + std::string(kTitleStr2) + + "Nav2" + + "" + + "", "text/html"); + + // Create the browser. + CreateBrowser(kTitleUrl1); + } + + virtual void OnTitleChange(CefRefPtr browser, + const CefString& title) OVERRIDE { + std::string title_str = title; + if (step_ == 0 || step_ == 2) { + EXPECT_STREQ(kTitleStr1, title_str.c_str()); + } else if (step_ == 1 || step_ == 3) { + EXPECT_STREQ(kTitleStr2, title_str.c_str()); + } else if (step_ == 4) { + EXPECT_STREQ(kTitleStr3, title_str.c_str()); + } + + got_title_[step_].yes(); + + if (step_ == 4) + DestroyTest(); + } + + virtual void OnLoadEnd(CefRefPtr browser, + CefRefPtr frame, + int httpStatusCode) OVERRIDE { + switch (step_++) { + case 0: + frame->LoadURL(kTitleUrl2); + break; + case 1: + browser->GoBack(); + break; + case 2: + browser->GoForward(); + break; + case 3: + frame->ExecuteJavaScript("setTitle()", kTitleUrl2, 0); + break; + default: + EXPECT_TRUE(false); // Not reached. + } + } + + private: + virtual void DestroyTest() { + for (int i = 0; i < 5; ++i) + EXPECT_TRUE(got_title_[i]) << "step " << i; + + TestHandler::DestroyTest(); + } + + int step_; + + TrackCallback got_title_[5]; +}; + +} // namespace + +// Test title notifications. +TEST(DisplayTest, Title) { + CefRefPtr handler = new TitleTestHandler(); + handler->ExecuteTest(); +}