Using WebContents::UpdateTitleForEntry() instead of NavigationEntry::SetTitle()

chrome/browser/task_management/providers/web_contents/guest_task.cc

 // Copyright 2015 The Chromium 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 "chrome/browser/task_management/providers/web_contents/guest_task.h"
 
 #include "components/guest_view/browser/guest_view_base.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ skipping 22 matching lines @@
   return Task::GUEST;
 }
 
 base::string16 GuestTask::GetCurrentTitle(
     content::WebContents* web_contents) const {
   DCHECK(web_contents);
 
   guest_view::GuestViewBase* guest =
       guest_view::GuestViewBase::FromWebContents(web_contents);
 
-  DCHECK(guest);
+  if (!guest)
+    return base::string16();

[Charlie Reis, 2016/06/24 22:21:17]

Just curious, in which case did this fail?  Might be nice to have a comment
about it, unless it's covered by one of your new tests.

[afakhry, 2016/06/27 14:29:18]

This caused crashes in some browsertests when I changed the task manager from
listening to DidNavigateMainFrame() to listening to DidFinishNavigation() as
Nick told me the former will be deprecated. Not related to interstitial titles.

Here are the failing tests:
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...

With the following stack trace:

[13384:13384:0623/120511:FATAL:guest_task.cc(43)] Check failed: guest.
#0 0x000004c7a1ee base::debug::StackTrace::StackTrace()
#1 0x000004c9024a logging::LogMessage::~LogMessage()
#2 0x000004e9fac7 task_management::GuestTask::GetCurrentTitle()
#3 0x000004e9fb6b task_management::GuestTask::UpdateTitle()
#4 0x000004ea2998 task_management::WebContentsEntry::DidFinishNavigation()
#5 0x0000025fa347 content::WebContentsImpl::DidFinishNavigation()
#6 0x000002364e18 content::NavigationHandleImpl::~NavigationHandleImpl()
#7 0x000002365199 content::NavigationHandleImpl::~NavigationHandleImpl()
#8 0x00000236ca4a content::RenderFrameHostImpl::~RenderFrameHostImpl()
#9 0x00000236ce49 content::RenderFrameHostImpl::~RenderFrameHostImpl()
#10 0x00000237d128 content::RenderFrameHostManager::~RenderFrameHostManager()
#11 0x000002356556 content::FrameTreeNode::~FrameTreeNode()
#12 0x000002356897 content::FrameTreeNode::ResetForNewProcess()
#13 0x0000025e895a content::WebContentsImpl::~WebContentsImpl()
#14 0x0000025e9739 content::WebContentsImpl::~WebContentsImpl()
#15 0x0000036472eb guest_view::GuestViewBase::Destroy()
#16 0x0000025e8fa4 content::WebContentsImpl::~WebContentsImpl()
#17 0x0000025e9739 content::WebContentsImpl::~WebContentsImpl()
#18 0x0000035b03e3 extensions::AppWindowContentsImpl::~AppWindowContentsImpl()
#19 0x0000035ad91e extensions::AppWindow::~AppWindow()
#20 0x0000035adad9 extensions::AppWindow::~AppWindow()
#21 0x0000035ae497 extensions::AppWindow::OnNativeClose()
#22 0x0000058aaa66 views::Widget::OnNativeWidgetDestroyed()
#23 0x0000058b66cc views::NativeWidgetAura::OnWindowDestroyed()
#24 0x0000061f629e aura::Window::~Window()
#25 0x0000061f69a9 aura::Window::~Window()
#26 0x000004d074a9 base::debug::TaskAnnotator::RunTask()
#27 0x000004c97a45 base::MessageLoop::RunTask()
#28 0x000004c97d38 base::MessageLoop::DeferOrRunPendingTask()
#29 0x000004c9805b base::MessageLoop::DoWork()
#30 0x000004c99b79 base::MessagePumpGlib::Run()
#31 0x000004c9757d base::MessageLoop::RunHandler()
#32 0x000004cba500 base::RunLoop::Run()
#33 0x0000075fe99c content::RunThisRunLoop()
#34 0x0000075fe93a content::RunMessageLoop()
#35 0x0000015a2741 InProcessBrowserTest::QuitBrowsers()
#36 0x0000015a24e5 InProcessBrowserTest::RunTestOnMainThreadLoop()
#37 0x0000016842f4 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#38 0x000004d38da9 ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#39 0x000004d37ea5 ChromeBrowserMainParts::PreMainMessageLoopRun()
#40 0x000001759e31
chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
#41 0x00000229a3d5 content::BrowserMainLoop::PreMainMessageLoopRun()
#42 0x0000025c3517 content::StartupTaskRunner::RunAllTasksNow()
#43 0x000002298b94 content::BrowserMainLoop::CreateStartupTasks()
#44 0x00000229d182 content::BrowserMainRunnerImpl::Initialize()
#45 0x000002296336 content::BrowserMain()
#46 0x000004b8149e content::ContentMainRunnerImpl::Run()
#47 0x000004b7fe80 content::ContentMain()
#48 0x000001683ca8 content::BrowserTestBase::SetUp()
#49 0x0000015a09eb InProcessBrowserTest::SetUp()
#50 0x0000057c2cc8 testing::Test::Run()
#51 0x0000057c3a3a testing::TestInfo::Run()
#52 0x0000057c3ec3 testing::TestCase::Run()
#53 0x0000057cb0b9 testing::internal::UnitTestImpl::RunAllTests()
#54 0x0000057cad5e testing::UnitTest::Run()
#55 0x0000015acb66 base::TestSuite::Run()
#56 0x000004c6f59b ChromeTestSuiteRunner::RunTestSuite()
#57 0x0000075fb7ce content::LaunchTests()
#58 0x0000005fedc1 main
#59 0x7f780ddf67ed __libc_start_main
#60 0x0000005fec81 <unknown>

I'm not sure of the meaning of that when it happens, so I don't know what's the
right comment to write here. I also changed it from returning an empty
string16() to returning the original cached title().

[Charlie Reis, 2016/06/28 00:47:09]

Ack.

For posterity (when the link above expires), the failing tests were:

WebViewTests_WebViewTest.Shim_TestNestedCrossOriginSubframes_0
WebViewTests_WebViewTest.Shim_TestNestedCrossOriginSubframes_1
WebViewTests_WebViewTest.Shim_TestNestedSubframes_0
WebViewTests_WebViewTest.Shim_TestNestedSubframes_1

 
   base::string16 title =
       l10n_util::GetStringFUTF16(guest->GetTaskPrefix(),
                                  RendererTask::GetTitleFromWebContents(
                                      web_contents));
 
   return title;
 }
 
 }  // namespace task_management