Fix a pair of ResourceLoader cancellation/error bugs.

content/browser/loader/intercepting_resource_handler_unittest.cc

Index: content/browser/loader/intercepting_resource_handler_unittest.cc
diff --git a/content/browser/loader/intercepting_resource_handler_unittest.cc b/content/browser/loader/intercepting_resource_handler_unittest.cc
index 9f2096ce76cc344fa15b98a40c0544d459a2b1a0..87ba506334844dd910b3549812e32a2b0545b694 100644
--- a/content/browser/loader/intercepting_resource_handler_unittest.cc
+++ b/content/browser/loader/intercepting_resource_handler_unittest.cc
@@ -233,6 +233,11 @@ TEST_F(InterceptingResourceHandlerTest, HandlerSwitchWithPayload) {
   std::string old_handler_body;
   std::unique_ptr<TestResourceHandler> old_handler_scoped(
       new TestResourceHandler(&old_handler_status, &old_handler_body));
+  // When sending a payload to the old ResourceHandler, the
+  // InterceptingResourceHandler doesn't send a final EOF read.
+  // TODO(mmenke):  Should it?  Or can we just get rid of that 0-byte read
+  // entirely?

[Randy Smith (Not in Mondays), 2016/12/02 20:38:55]

I'm assuming you have some way of tracking all of these corner cases and making
sure they don't get dropped on the floor?  (This is a transformation of a "file
a bug?" request that was my knee jerk reflex--I don't actually think that's the
right answer, since there are so many of these small things, but I would like to
make sure we don't lose track of them.  And TODOs are better for code archeology
than actually remembering to come back to something.)

[mmenke, 2016/12/02 21:26:52]

Erm...Not really.  I think there's a high chance of this getting dropped on the
floor, actually, unless I figure out a good way to go the removal route (The
problem with removing is the mime sniffer - it needs to know when things are
done to delay teardown.  It could just delay teardown instead, like the
RedirectToFileHandler does, though if I had to choose between the two, I'd go
with delaying the 0-byte read).

[Randy Smith (Not in Mondays), 2016/12/06 20:46:44]

How would you feel about creating a bug something like "clean up sharp edges on
the ResourceHandler API" and just use it to store notes about this sorta stuff? 
Or dumping things like this you run into into one of the bugs you already have?

+  old_handler_scoped->set_expect_eof_read(false);
   TestResourceHandler* old_handler = old_handler_scoped.get();
   scoped_refptr<net::IOBuffer> old_buffer = old_handler->buffer();
   std::unique_ptr<InterceptingResourceHandler> intercepting_handler(
@@ -600,6 +605,11 @@ TEST_F(InterceptingResourceHandlerTest, DeferredOperations) {
   std::string old_handler_body;
   std::unique_ptr<TestResourceHandler> old_handler(
       new TestResourceHandler(&old_handler_status, &old_handler_body));
+  // When sending a payload to the old ResourceHandler, the
+  // InterceptingResourceHandler doesn't send a final EOF read.
+  // TODO(mmenke):  Should it?  Or can we just get rid of that 0-byte read
+  // entirely?
+  old_handler->set_expect_eof_read(false);
   old_handler->SetBufferSize(10);
   old_handler->set_defer_on_read_completed(true);
 
@@ -687,6 +697,10 @@ TEST_F(InterceptingResourceHandlerTest, DeferredOperations) {
   EXPECT_EQ(old_handler_status.status(), net::URLRequestStatus::SUCCESS);
   EXPECT_EQ(new_handler_status.status(), net::URLRequestStatus::IO_PENDING);
 
+  // Final EOF byte is read.
+  ASSERT_TRUE(intercepting_handler->OnReadCompleted(0, &defer));
+  ASSERT_FALSE(defer);
+
   defer = false;
   intercepting_handler->OnResponseCompleted({net::URLRequestStatus::SUCCESS, 0},
                                             &defer);
@@ -760,6 +774,11 @@ TEST_F(InterceptingResourceHandlerTest, CancelNewHandler) {
   std::string old_handler_body;
   std::unique_ptr<TestResourceHandler> old_handler(
       new TestResourceHandler(&old_handler_status, &old_handler_body));
+  // When sending a payload to the old ResourceHandler, the
+  // InterceptingResourceHandler doesn't send a final EOF read.
+  // TODO(mmenke):  Should it?  Or can we just get rid of that 0-byte read
+  // entirely?
+  old_handler->set_expect_eof_read(false);
 
   std::unique_ptr<InterceptingResourceHandler> intercepting_handler(
       new InterceptingResourceHandler(std::move(old_handler), request.get()));