Fix ExpiredCertAndGoBackTests

content/browser/ssl/ssl_error_handler.cc

Index: content/browser/ssl/ssl_error_handler.cc
diff --git a/content/browser/ssl/ssl_error_handler.cc b/content/browser/ssl/ssl_error_handler.cc
index e3eceabddc6fdc51013e0f3da97ad09c0a3a8b8b..fc448382508dac17d340215892312d6ff8ae7ace 100644
--- a/content/browser/ssl/ssl_error_handler.cc
+++ b/content/browser/ssl/ssl_error_handler.cc
@@ -135,7 +135,7 @@ void SSLErrorHandler::CompleteCancelRequest(int error) {
 
   net::URLRequest* request =
       resource_dispatcher_host_->GetURLRequest(request_id_);
-  if (request) {
+  if (request && request->is_pending()) {

[darin (slow to review), 2012/02/27 19:26:25]

oh, hmm...

have you considered cancelling the request via
ResourceDispatcherHost::CancelRequest?  requests start out their life possibly
being deferred until some time.  they are not-yet-pending during this time.

if you don't cancel the request here, then the ResourceDispatcherHost may still
start the request later.  does that matter to you?

[tpayne, 2012/02/27 19:30:51]

I haven't seen any instances where the request had not yet started. All the
instances I've looked at (not that many, granted) were cases where the request
had already been notified.

[wtc, 2012/02/28 20:02:12]

tpayne: do you know why the request may be not pending at
this point?  Because the request has been canceled by
someone else in the interim?

darin: I believe the request must have been started if it
has an SSL error.  So it is impossible for the request to
be not-yet-pending here.

[tpayne, 2012/02/28 21:53:58]

The state that is causing the flakiness is request is no-longer-pending. I
wasn't able to determine what is canceling the request before we get here. I'll
do a bit more digging and see if I can come up with a culprit.

[tpayne, 2012/02/29 01:15:11]

The original cancel is occurring with this stack:

	net::URLRequest::NotifyRequestCompleted() [0x18f4bf7]
	net::URLRequest::DoCancel() [0x18f300e]
	net::URLRequest::Cancel() [0x18f2c4b]
	ResourceDispatcherHost::CancelRequestInternal() [0x3c48f1e]
	ResourceDispatcherHost::CancelRequest() [0x3c48c89]
	ResourceDispatcherHost::OnCancelRequest() [0x3c44ed5]
	DispatchToMethod<>() [0x3c51c75]
	ResourceHostMsg_CancelRequest::Dispatch<>() [0x3c4f8b4]
	ResourceDispatcherHost::OnMessageReceived() [0x3c43787]
	ResourceMessageFilter::OnMessageReceived() [0x3c5c563]
	content::BrowserMessageFilter::DispatchMessage() [0x3cc785b]
	content::BrowserMessageFilter::OnMessageReceived() [0x3cc76f8]
	IPC::ChannelProxy::Context::TryFilters() [0x25f85b3]
	IPC::ChannelProxy::Context::OnMessageReceived() [0x25f8637]
	IPC::Channel::ChannelImpl::ProcessIncomingMessages() [0x25f49b9]
	IPC::Channel::ChannelImpl::OnFileCanReadWithoutBlocking() [0x25f6881]
	base::MessagePumpLibevent::FileDescriptorWatcher::OnFileCanReadWithoutBlocking()
[0x1673a2c]
	base::MessagePumpLibevent::OnLibeventNotification() [0x16759ff]
	event_process_active [0x1e438d9]
	event_base_loop [0x1e43c08]
	base::MessagePumpLibevent::Run() [0x1674fa6]
	MessageLoop::RunInternal() [0x16b8ce3]
	MessageLoop::RunHandler() [0x16b8b96]
	MessageLoop::Run() [0x16b84cb]
	base::Thread::Run() [0x17088b2]
	base::Thread::ThreadMain() [0x1708a32]
	base::(anonymous namespace)::ThreadFunc() [0x16fefb3]
	start_thread [0x7fc82dfbb9ca]
	0x7fc82804a70d


The (flakily) failing attempt to cancel is occuring with this stack:


	SSLErrorHandler::CancelRequest() [0x3d83b10]
	SSLPolicy::OnAllowCertificate() [0x3c7ae77]
	base::internal::RunnableAdapter<>::Run() [0x3c7b76c]
	base::internal::InvokeHelper<>::MakeItSo() [0x3c7b674]
	base::internal::Invoker<>::Run() [0x3c7b5be]
	base::Callback<>::Run() [0xbab004]
	SSLBlockingPage::NotifyDenyCertificate() [0x43e0d75]
	SSLBlockingPage::OnDontProceed() [0x43e0d38]
	InterstitialPageImpl::DontProceed() [0x3c7e31d]
	NavigationControllerImpl::NavigateToPendingEntry() [0x3c85ff7]
	NavigationControllerImpl::GoBack() [0x3c82ab9]
	Browser::GoBack() [0xe3b0a4]
	SSLUITest_DISABLED_TestHTTPSExpiredCertAndGoBackViaButton_Test::RunTestOnMainThread()
[0x66d80e]
	InProcessBrowserTest::RunTestOnMainThreadLoop() [0x165a8d8]
	BrowserTestBase::ProxyRunTestOnMainThreadLoop() [0x436906d]
	base::internal::RunnableAdapter<>::Run() [0x4369385]
	base::internal::InvokeHelper<>::MakeItSo() [0x436931b]
	base::internal::Invoker<>::Run() [0x43692d3]
	base::Callback<>::Run() [0x94286b]
	ChromeBrowserMainParts::PreMainMessageLoopRunImpl() [0x43fa142]
	ChromeBrowserMainParts::PreMainMessageLoopRun() [0x43f8f96]
	content::BrowserMainLoop::CreateThreads() [0x3b0839e]
	(anonymous namespace)::BrowserMainRunnerImpl::Initialize() [0x3ccd93f]
	BrowserMain() [0x4a3e21f]
	BrowserTestBase::SetUp() [0x4369020]
	InProcessBrowserTest::SetUp() [0x1659fc1]
	testing::internal::HandleSehExceptionsInMethodIfSupported<>() [0x1b3f09a]
	testing::internal::HandleExceptionsInMethodIfSupported<>() [0x1b3c71e]
	testing::Test::Run() [0x1b31b01]
	testing::TestInfo::Run() [0x1b3237a]
	testing::TestCase::Run() [0x1b32a70]
	testing::internal::UnitTestImpl::RunAllTests() [0x1b37847]
	testing::internal::HandleSehExceptionsInMethodIfSupported<>() [0x1b3fec1]
	testing::internal::HandleExceptionsInMethodIfSupported<>() [0x1b3cecd]
	testing::UnitTest::Run() [0x1b36364]
	base::TestSuite::Run() [0x1737df9]
	ChromeTestLauncherDelegate::RunTestSuite() [0x7ef257]
	test_launcher::LaunchTests() [0x862f68]
	main [0x7ef05d]
	0x7fc827f82c4d
	0x43c539

[tpayne, 2012/02/29 01:58:24]

One more level up, it turns out that NavigateToPendingEntry has 2 paths that
cause the request to be cancelled:

TabContents::Stop triggers NotifyRequestCompleted and
InterstitialPageImpl::DontProceed triggers SSLErrorHandler::CancelRequest. If
NotifyRequestCompleted happens first, we get the failure due to is_pending_
getting cleared. Hope that helps.

     // The request can be NULL if it was cancelled by the renderer (as the
     // result of the user navigating to a new page from the location bar).
     DVLOG(1) << "CompleteCancelRequest() url: " << request->url().spec();