Revert "Always use backpressure for fetch"

This reverts commit e0eac774.

Reason for revert: this patch breaks DevTools protocol, see crbug.com/807483

Original change's description:
> Always use backpressure for fetch
> 
> Previously, backpressure would only be applied to fetch responses if the
> Cache-Control: no-store header was set. This was to avoid blocking other
> resource loads that might have to wait for the lock on the http disk
> cache that was held by the resource load to which backpressure was being
> applied.
> 
> As of https://chromium-review.googlesource.com/684615 the disk cache
> supports multiple concurrent writers and so this protection is no longer
> needed.
> 
> Remove support for the non-backpressure mode and always apply
> backpressure.
> 
> The fact that the body is no longer implicitly read is visible
> to tests that observe the result of the fetch() via a side
> channel, specifically inspector and browser tests. Fix such tests
> to explicitly read the body to completion.
> 
> Additionally, in order to satisfy the presubmit check, make
> SharedMemoryDataConsumerHandle::Context::on_reader_detached_ a
> OnceClosure.
> 
> The implementation already implicitly ran on_reader_detached_ at most
> once, because is_on_reader_detached_valid_ would be set to false by
> ResetOnReaderDetached immediately after PostTask was called. Now this
> has been made explicit by using OnceClosure.
> 
> on_reader_detached_ is only reset on the writer_task_runner_
> thread. When the closure is not run this is implemented by the logic in
> ResetOnReaderDetached(), the same as before. When the closure is run the
> behaviour has changed slightly: the OnceClosure is moved into the task
> that is posted to the writer_task_runner_ thread, where it will be
> destroyed automatically after running. There's no nead for a separate
> reset in this case.
> 
> BUG=535793
> 
> Change-Id: I9d2b58e183b84640afc153ac99de08fdc2368240
> Reviewed-on: https://chromium-review.googlesource.com/861695
> Commit-Queue: Adam Rice <ricea@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#530070}

TBR=ricea@chromium.org,yhirano@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 535793
Change-Id: I79a176496f6fada40ba8f2e4d95848421809ecee
Reviewed-on: https://chromium-review.googlesource.com/894683Reviewed-by: Andrey Lushnikov <lushnikov@chromium.org>
Commit-Queue: Andrey Lushnikov <lushnikov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533153}

chrome/test/data/password/password_fetch_submit.html

@@ -22,11 +22,10 @@ function send_fetch() {
           // Pretend like auth succeeded by hiding the login and signup forms.
           document.getElementById("testform").style.display = "none";
           document.getElementById("signup_testform").style.display = "none";
-          // Finish loading the body of the response and then give
-          // PasswordAutofillAgent a chance to run before upload. It will run as
-          // soon as JS execution ends, so this shouldn't introduce any timing
-          // dependent flakes.
-          response.arrayBuffer().then(() => setTimeout(delayedUpload, 0));
+          // Delay upload so that handler in PasswordAutofillAgent can be run
+          // first. This will happen immediately after JS execution ends, so
+          // this shouldn't introduce any timing dependent flakes.
+          setTimeout(delayedUpload, 0);
         }
       }
     )

content/renderer/loader/shared_memory_data_consumer_handle.cc

@@ -54,12 +54,12 @@ using Result = blink::WebDataConsumerHandle::Result;
 class SharedMemoryDataConsumerHandle::Context final
     : public base::RefCountedThreadSafe<Context> {
  public:
-  explicit Context(base::OnceClosure on_reader_detached)
+  explicit Context(const base::Closure& on_reader_detached)
       : result_(kOk),
         first_offset_(0),
         client_(nullptr),
         writer_task_runner_(base::ThreadTaskRunnerHandle::Get()),
-        on_reader_detached_(std::move(on_reader_detached)),
+        on_reader_detached_(on_reader_detached),
         is_on_reader_detached_valid_(!on_reader_detached_.is_null()),
         is_handle_active_(true),
         is_two_phase_read_in_progress_(false) {}
@@ -76,9 +76,7 @@ class SharedMemoryDataConsumerHandle::Context final
         // We post a task even in the writer thread in order to avoid a
         // reentrance problem as calling |on_reader_detached_| may manipulate
         // the context synchronously.
-        is_on_reader_detached_valid_ = false;
-        writer_task_runner_->PostTask(FROM_HERE,
-                                      std::move(on_reader_detached_));
+        writer_task_runner_->PostTask(FROM_HERE, on_reader_detached_);
       }
       Clear();
     }
@@ -252,7 +250,7 @@ class SharedMemoryDataConsumerHandle::Context final
   Client* client_;
   scoped_refptr<base::SingleThreadTaskRunner> notification_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> writer_task_runner_;
-  base::OnceClosure on_reader_detached_;
+  base::Closure on_reader_detached_;
   // We need this boolean variable to remember if |on_reader_detached_| is
   // callable because we need to reset |on_reader_detached_| only on the writer
   // thread and hence |on_reader_detached_.is_null()| is untrustworthy on
@@ -265,8 +263,10 @@ class SharedMemoryDataConsumerHandle::Context final
 };
 
 SharedMemoryDataConsumerHandle::Writer::Writer(
-    const scoped_refptr<Context>& context)
-    : context_(context) {}
+    const scoped_refptr<Context>& context,
+    BackpressureMode mode)
+    : context_(context), mode_(mode) {
+}
 
 SharedMemoryDataConsumerHandle::Writer::~Writer() {
   Close();
@@ -290,8 +290,14 @@ void SharedMemoryDataConsumerHandle::Writer::AddData(
     }
 
     needs_notification = context_->IsEmpty();
-    context_->Push(
-        std::make_unique<DelegateThreadSafeReceivedData>(std::move(data)));
+    std::unique_ptr<RequestPeer::ThreadSafeReceivedData> data_to_pass;
+    if (mode_ == kApplyBackpressure) {
+      data_to_pass =
+          std::make_unique<DelegateThreadSafeReceivedData>(std::move(data));
+    } else {
+      data_to_pass = std::make_unique<FixedReceivedData>(data.get());
+    }
+    context_->Push(std::move(data_to_pass));
   }
 
   if (needs_notification) {
@@ -430,14 +436,16 @@ Result SharedMemoryDataConsumerHandle::ReaderImpl::EndRead(size_t read_size) {
 }
 
 SharedMemoryDataConsumerHandle::SharedMemoryDataConsumerHandle(
+    BackpressureMode mode,
     std::unique_ptr<Writer>* writer)
-    : SharedMemoryDataConsumerHandle(base::OnceClosure(), writer) {}
+    : SharedMemoryDataConsumerHandle(mode, base::Closure(), writer) {}
 
 SharedMemoryDataConsumerHandle::SharedMemoryDataConsumerHandle(
-    base::OnceClosure on_reader_detached,
+    BackpressureMode mode,
+    const base::Closure& on_reader_detached,
     std::unique_ptr<Writer>* writer)
-    : context_(new Context(std::move(on_reader_detached))) {
-  writer->reset(new Writer(context_));
+    : context_(new Context(on_reader_detached)) {
+  writer->reset(new Writer(context_, mode));
 }
 
 SharedMemoryDataConsumerHandle::~SharedMemoryDataConsumerHandle() {

content/renderer/loader/shared_memory_data_consumer_handle.h

@@ -25,9 +25,14 @@ class CONTENT_EXPORT SharedMemoryDataConsumerHandle final
   class Context;
 
  public:
+  enum BackpressureMode {
+    kApplyBackpressure,
+    kDoNotApplyBackpressure,
+  };
+
   class CONTENT_EXPORT Writer final {
    public:
-    explicit Writer(const scoped_refptr<Context>& context);
+    Writer(const scoped_refptr<Context>& context, BackpressureMode mode);
     ~Writer();
     // Note: Writer assumes |AddData| is not called in a client's didGetReadable
     // callback. There isn't such assumption for |Close| and |Fail|.
@@ -38,6 +43,7 @@ class CONTENT_EXPORT SharedMemoryDataConsumerHandle final
 
    private:
     scoped_refptr<Context> context_;
+    BackpressureMode mode_;
 
     DISALLOW_COPY_AND_ASSIGN(Writer);
   };
@@ -63,13 +69,15 @@ class CONTENT_EXPORT SharedMemoryDataConsumerHandle final
 
   // Creates a handle and a writer associated with the handle. The created
   // writer should be used on the calling thread.
-  explicit SharedMemoryDataConsumerHandle(std::unique_ptr<Writer>* writer);
+  SharedMemoryDataConsumerHandle(BackpressureMode mode,
+                                 std::unique_ptr<Writer>* writer);
   // |on_reader_detached| will be called aynchronously on the calling thread
   // when the reader (including the handle) is detached (i.e. both the handle
   // and the reader are destructed). The callback will be reset in the internal
   // context when the writer is detached, i.e. |Close| or |Fail| is called,
   // and the callback will never be called.
-  SharedMemoryDataConsumerHandle(base::OnceClosure on_reader_detached,
+  SharedMemoryDataConsumerHandle(BackpressureMode mode,
+                                 const base::Closure& on_reader_detached,
                                  std::unique_ptr<Writer>* writer);
   ~SharedMemoryDataConsumerHandle() override;
 

content/renderer/loader/web_url_loader_impl.cc

@@ -852,8 +852,15 @@ void WebURLLoaderImpl::Context::OnReceivedResponse(
   }
 
   if (use_stream_on_response_) {
+    SharedMemoryDataConsumerHandle::BackpressureMode mode =
+        SharedMemoryDataConsumerHandle::kDoNotApplyBackpressure;
+    if (info.headers &&
+        info.headers->HasHeaderValue("Cache-Control", "no-store")) {
+      mode = SharedMemoryDataConsumerHandle::kApplyBackpressure;
+    }
+
     auto read_handle = std::make_unique<SharedMemoryDataConsumerHandle>(
-        base::BindOnce(&Context::CancelBodyStreaming, this),
+        mode, base::Bind(&Context::CancelBodyStreaming, this),
         &body_stream_writer_);
 
     // Here |body_stream_writer_| has an indirect reference to |this| and that

third_party/WebKit/LayoutTests/http/tests/devtools/extensions/resources/extensions-network.html

@@ -10,8 +10,7 @@ p { font-family: 'test'; }
 <img src="missing-image.png">
 <script>
 function doXHR() {
-  return fetch('extensions-network.html')
-    .then(response => response.arrayBuffer());
+  return fetch('extensions-network.html');
 }
 </script>
 <p>some text.</p>

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/audits/get-encoded-response.js

@@ -11,8 +11,7 @@
   async function logResponse(url, encoding, quality, sizeOnly) {
     testRunner.log(`\nResults for ${url} encoding=${encoding} q=${quality} sizeOnly=${sizeOnly}`);
 
-    session.evaluate(`fetch(${JSON.stringify(url)})
-                        .then(response => response.arrayBuffer())`);
+    session.evaluate(`fetch(${JSON.stringify(url)})`);
 
     const requestId = (await dp.Network.onceResponseReceived()).params.requestId;
     const result = (await dp.Audits.getEncodedResponse({requestId, encoding, quality, sizeOnly})).result;

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network-data-length.js

@@ -10,9 +10,7 @@
   var pendingRequests = 0;
 
   function sendRequest(url) {
-    dp.Runtime.evaluate({expression: `
-      fetch('${url}')
-        .then(response => response.arrayBuffer())`});
+    dp.Runtime.evaluate({expression: `fetch('${url}')`});
     pendingRequests++;
   }
 

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network/get-response-body.js

@@ -5,8 +5,7 @@
   await dp.Network.enable();
 
   async function logResponseBody(url) {
-    session.evaluate(`fetch(${JSON.stringify(url)})
-                        .then(response => response.arrayBuffer())`);
+    session.evaluate(`fetch(${JSON.stringify(url)});`);
 
     var requestWillBeSent = (await dp.Network.onceRequestWillBeSent()).params;
     testRunner.log(`Request for ${requestWillBeSent.request.url}`);

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network/override-referer-with-downgrade.js

@@ -5,8 +5,7 @@
 
   dp.Network.enable();
   await dp.Network.setExtraHTTPHeaders({headers: {'ReFeReR': 'https://127.0.0.1:8000/'}});
-  session.evaluate(`fetch('${testRunner.url('./resources/echo-headers.php?headers=HTTP_REFERER')}')
-                      .then(response => response.arrayBuffer())`);
+  session.evaluate(`fetch('${testRunner.url('./resources/echo-headers.php?headers=HTTP_REFERER')}')`);
 
   var response = (await dp.Network.onceLoadingFinished()).params;
   var content = await dp.Network.getResponseBody({requestId: response.requestId});

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network/request-interception-on-both-redirect-rewrite.js

@@ -18,8 +18,7 @@
   ]});
   testRunner.log('Request interception patterns sent.');
 
-  session.evaluate(`fetch('${testRunner.url('../resources/redirect1.php')}')
-                      .then(response => response.arrayBuffer())`);
+  session.evaluate(`fetch('${testRunner.url('../resources/redirect1.php')}')`);
 
   // Should be redirect1.php as request.
   var interceptionEvent = await waitForInterceptionEvent();

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network/request-interception-on-response-redirect.js

@@ -65,8 +65,7 @@
   await new Promise(resolve => {
     session.protocol.Network.onResponseReceived(resolve);
     session.evaluate(`
-      fetch('${testRunner.url('../resources/ping-redirect.php')}')
-        .then(response => response.arrayBuffer());
+      fetch('${testRunner.url('../resources/ping-redirect.php')}');
     `);
   });
 

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network/request-interception-patterns.js

@@ -71,14 +71,11 @@
    * @return {!Promise}
    */
   async function testUrls() {
-    session.evaluate(`fetch('../network/resources/small-test-1.txt')
-                        .then(response => response.arrayBuffer())`);
+    session.evaluate(`fetch('../network/resources/small-test-1.txt')`);
     await new Promise(resolve => responseWasReceivedCallback = resolve);
-    session.evaluate(`fetch('../network/resources/small-test-2.txt')
-                        .then(response => response.arrayBuffer())`);
+    session.evaluate(`fetch('../network/resources/small-test-2.txt')`);
     await new Promise(resolve => responseWasReceivedCallback = resolve);
-    session.evaluate(`fetch('../resources/test-page.html')
-                        .then(response => response.arrayBuffer())`);
+    session.evaluate(`fetch('../resources/test-page.html')`);
     await new Promise(resolve => responseWasReceivedCallback = resolve);
     testRunner.log('');
   }

third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network/search-in-response.js

@@ -4,8 +4,7 @@
   await dp.Network.enable();
 
   var url = testRunner.url('./resources/final.js');
-  session.evaluate(`fetch("${url}")
-                      .then(response => response.arrayBuffer());`);
+  session.evaluate(`fetch("${url}");`);
 
   var requestWillBeSent = (await dp.Network.onceRequestWillBeSent()).params;
   testRunner.log(`Request for ${requestWillBeSent.request.url}`);