Update MimeSniffingResourceHandler tests to use MockResourceLoader.

content/browser/loader/mime_sniffing_resource_handler.cc

 // Copyright (c) 2012 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 "content/browser/loader/mime_sniffing_resource_handler.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 227 matching lines @@
   }
 
   // If the BufferingHandler is acting as a pass-through handler, just ask the
   // upwards ResourceController to resume the request.
   if (state_ == STATE_STARTING || state_ == STATE_STREAMING) {
     controller()->Resume();
     return;
   }
 
   // Otherwise proceed with the replay of the response. If it is successful,
-  // it will resume the request.
-  AdvanceState();
+  // it will resume the request. Posted as a task to avoid re-entrancy into
+  // the calling class.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&MimeSniffingResourceHandler::AdvanceState,
+                            weak_ptr_factory_.GetWeakPtr()));

[Randy Smith (Not in Mondays), 2017/01/12 18:44:29]

Hmmm.  Your top level comment is that this PostTasks in exactly the cases in
which that's needed to avoid re-entrancy.  But it seems to me that if
MimeSniffingResourceHandler is acting in a pass through mode (or in another mode
where the only effect of AdvanceState() is to pass the Resume() up the stack)
that this'll be an extra PostTask.  I .... am concerned that that's a
performance hit exactly where we don't want it (i.e. on every request whenever
the request is paused and then resumed--for large requests that might be
frequently).  WDYT?

[mmenke, 2017/01/12 19:04:01]

I think what I said is correct, but it's not exactly obvious.

When we call AdvanceState, it will call ProcessState.  We call one of
MaybeIntercept, ReplayResponseReceived, or ReplayReadCompleted and they don't
defer.

So ReplayResponseReceived/ReplayReadCompleted always call into next_handler_, so
are clearly not safe to call synchronously.

That leaves MaybeIntercept.  If we're BUFFERING, we're reading data from the
server and not sending it to next_handler_...So Resume() can't be called when
we're in that state, and it's a moot point if we can re-entrantly call into
next_handler_ or not.  However, even if it could happen, we're only in
STATE_BUFFERING when reading data, so when we advance state, we're going to have
to replace that data (Even if it's just an EOF).

Or am I missing something?

[mmenke, 2017/01/12 19:06:32]

(Also note that in pass through mode, we call resume the request just above the
post task, so we only have to think about the case where we switch to pass
through mode in the AdvanceState call)

[Randy Smith (Not in Mondays), 2017/01/12 19:08:58]

*bonk*  That's what I was missing.  Thank you.  LGTM.

[mmenke, 2017/01/12 19:17:15]

Note that there was a bug in my argument about the STATE_BUFFERING case - if it
wants more data, it completes synchronously.  But the "Resume not called while
buffering" argument still holds.  Anyways, CQing.

 }
 
 void MimeSniffingResourceHandler::Cancel() {
   controller()->Cancel();
 }
 
 void MimeSniffingResourceHandler::CancelAndIgnore() {
   controller()->CancelAndIgnore();
 }
 
@@ skipping 247 matching lines @@
 
 void MimeSniffingResourceHandler::OnPluginsLoaded(
     const std::vector<WebPluginInfo>& plugins) {
   // No longer blocking on the plugins being loaded.
   request()->LogUnblocked();
   if (state_ == STATE_BUFFERING)
     AdvanceState();
 }
 
 }  // namespace content