As per comment #0 Assigning to @leon.han@intel.com  for further action on this and providing the bisect information.

You are probably looking for a change made after 551340 (known good), but no later than 551350 (first known bad).

ChangeLog:
https://chromium.googlesource.com/chromium/src/+log/fcdb3ef9ea5fb65d0e92cdbcca01fad43fd7db56..2a911d93f01e3e9c7435c49df91b5641c4948098

Suspect: https://chromium.googlesource.com/chromium/src/+/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5

Reviewed-on: https://chromium-review.googlesource.com/967863

leon.han@intel.com:Please confirm the issue and help in re-assigning if it is not related to your change.

Thanks!

MEGA site doesn't seem to use service workers.

I suspect it's rather 

Fix parsing of segmented WebRequest formData
https://chromium.googlesource.com/chromium/src/+/afccef39a889d2659ca7cdee3dd2b59c3b7adc3d

The site seems to be using <input type=file>.

The site is indeed using <input>'s `File`s, it does makes use of `FileReader` to read chunks of type `ArrayBuffer` & `ulBlockExtraSize` length and sends each of them to a Worker for encryption, on completion those encrypted chunks are then sent to the server with a call to `XMLHttpRequest.send(theArrayBufferChunk)`

@falken are you sure this is a straight file form post?  From the little I could gather, it looks like Mega splits the file into chunks, encrypts them and POSTs one block at a time (I'm guessing ulmanager.ulBlockExtraSize in the repro instructions controls chunking).

@dcasorran do you have any extensions installed which may be using the webRequest API to intercept post data?  Can you repro with all extensions disabled?

Also, can you expand on how Chrome hangs/freezes?  Presumably it's different from mega.nz getting stuck in "Initializing" (see below).

I'm having trouble reproducing the issue - for me, mega.nz just hangs "Initializing" 9 times out of 10, regardless of browser/version.  I wish there was a more straightforward test, but I'll keep trying.

Hi @fmalita, it's a clean Canary installation with no extensions that i merely use for testing purposes, it does hangs/freezes the whole browser with the usual SWOD, although eventually it kinda resumes and allows me to switch to other tabs, but then it hangs again, basically.

Btw, the issue does start to happen after having uploaded ~4MB, since the site does first upload smaller chunks (that causes no issues) and then moves to the bigger chunks whose length is specified by `ulBlockExtraSize`

So, if you're facing transient connectivity issues being stuck on "Initializing" you won't be able to reproduce it yet.

Thanks for the details.  I still cannot repro the freeze, but I can now test uploading and have found a few things... 

We have three conditions compounding each-other:

  1) SharedBuffer splits flat data into 4K segments [1]

  2) ChromeExtensionsNetworkDelegateImpl always constructs a WebRequestInfo, even when there are no extensions registered for chrome.webRequest [2]

  3) WebRequestInfo's constructor attempts to parse the request form data, but it needs a flattened view -- so it reconstructs the contiguous buffer by appending to an std::string [3]


[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/shared_buffer.cc?rcl=9942d2112e8631dc43d3e6f3531b2fca9d5bbcfa&l=58
[2] https://cs.chromium.org/chromium/src/chrome/browser/net/chrome_extensions_network_delegate.cc?rcl=2f131cfdd01d6b8326fa467b8b7a9317bd1c5a1c&l=196
[3] https://cs.chromium.org/chromium/src/extensions/browser/api/web_request/web_request_info.cc?rcl=9942d2112e8631dc43d3e6f3531b2fca9d5bbcfa&l=278

So for a given ArrayBuffer block, we

 -- split into individually-allocated/copied 4K segments
 -- append one-at-a-time to string
 -- attempt to parse that string as formData for chrome.webRequest
 -- all this just to throw everything away 'cause there are no webRequest-registered extensions

afccef39a889d2659ca7cdee3dd2b59c3b7adc3d is responsible for #3 above.  It actually fixes another regression related to data segmentation: we used to flatten SharedBuffers further up the stack, but then we started deferring until needed (https://chromium-review.googlesource.com/c/chromium/src/+/581748).  So at a high level the behavior should be equivalent, modulo the current inefficient flattening process (appending to a string, 4K at a time), which at the time I thought would only trigger for chrome.webRequest extensions.

I suspect what's going on is #3 introduces substantial overhead for large block sizes, increases memory allocation pressure and can trigger SWOD if the system is resource-constrained.

I'll look at reducing the flattening overhead (we should at least preallocate the buffer), but the easiest change would probably be fixing #1 (which makes sense in general: when we start with flat/known-size data, there is no reason to segment it).

Also, someone familiar with chrome.webRequest should prolly investigate #2, 'cause it seems we a doing some non-trivial work to parse formData for every request, even when no extension is using this data.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce42a9d24c7aac91d073851e2ed4fc3180a8384d

commit ce42a9d24c7aac91d073851e2ed4fc3180a8384d
Author: Florin Malita <fmalita@chromium.org>
Date: Tue Jun 26 12:52:47 2018

Avoid unnecessary SharedBuffer data segmentation

When constructing SharedBuffers from flat data, we currently defer to
SharedBuffer::AppendInternal() -- which splits the source into segments.

This is unnecessary overhead (because SharedBuffer can already track a
single flat slab in buffer_), and can generate even more churn
downstream for clients which require a flattened data view (we're
effectively splitting then reassembling the data source for no reason).

Update SharedBuffer's flat data constructors to store all the data
contiguously in buffer_ (similar to SharedBuffer::AdoptVector).

Bug:  853500 
Change-Id: I523f23859e62c76225c8d5345005c5bd7911d9c3
Reviewed-on: https://chromium-review.googlesource.com/1113661
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570385}
[modify] https://crrev.com/ce42a9d24c7aac91d073851e2ed4fc3180a8384d/third_party/blink/renderer/platform/shared_buffer.cc
[modify] https://crrev.com/ce42a9d24c7aac91d073851e2ed4fc3180a8384d/third_party/blink/renderer/platform/shared_buffer_test.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/99ef4c2a857b7e875f3689161493a0e8c508df55

commit 99ef4c2a857b7e875f3689161493a0e8c508df55
Author: Florin Malita <fmalita@chromium.org>
Date: Thu Jun 28 12:21:34 2018

Flatten ResourceRequestBody data

[1] updated web_url_request_util.cc:GetRequestBodyForWebHTTPBody() to use a WebData
chunk iterator and push data to network::ResourceRequestBody one segment at a time.

This introduced an issue downstream, in WebRequest's form data parser, where data was
assumed to be contiguous.  The issue was addressed in [2], by adding a non-trivial
buffering mechanism in ParsedDataPresenter.  Unfortunately, the incremental buffering
seems to be problematic with large data payloads (blamed in the linked bug).

Circling back to the original change, I noticed ResourceRequestBody::AppendBytes()
does not perform any internal data consolidation, but in turn allocates and copies
one segment at a time.  Since the data is being copied anyway, there is no benefit to
preserving segmentation: we might as well make a consolidating copy on the spot, and
pass that to ResourceRequestBody.

I propose the following in this change:

  - update SharedBuffer::Copy (currently only used in tests) to support both
    WTF::Vector and std::vector specializations (in order to be usable outside Blink)
  - expose Copy on WebData
  - add std::vector-moving variants of ResourceRequestBody::AppendBytes() and
    DataElement::SetToBytes()
  - (*) change GetRequestBodyForWebHTTPBody() to make an upfront/consolidating copy,
    and pass (move) the data to ResourceRequestBody as a single chunk
  - revert the ParsedDataPresenter buffering workaround introduced in [2]

Essentially, this reverts the ResourceRequestBody segmentation changes introduced
in [1] & [2], by making the data copy explicit.

(*) <- this is the gist of the change, everything else is supporting or revert.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/581748
[2] https://chromium-review.googlesource.com/c/chromium/src/+/1011322

Bug:  853500 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;luci.chromium.try:linux_mojo;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I5f8d4bbaf7cc4b6372e868b4e62f6077bdf7d5a1
Reviewed-on: https://chromium-review.googlesource.com/1114258
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571089}
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/content/renderer/loader/web_url_request_util.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/extensions/browser/api/web_request/upload_data_presenter.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/extensions/browser/api/web_request/upload_data_presenter.h
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/extensions/browser/api/web_request/upload_data_presenter_unittest.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/services/network/public/cpp/data_element.h
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/services/network/public/cpp/resource_request_body.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/services/network/public/cpp/resource_request_body.h
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/public/platform/web_data.h
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/core/frame/frame_serializer_test.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/core/page/page_popup_client_test.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/core/paint/paint_property_tree_builder_test.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/exported/web_data.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/graphics/deferred_image_decoder_test.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/graphics/deferred_image_decoder_test_wo_platform.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/image-decoders/gif/gif_image_decoder_test.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/image-decoders/ico/ico_image_decoder_test.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/image-decoders/image_decoder_test_helpers.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/image-decoders/webp/webp_image_decoder_test.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/shared_buffer.cc
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/shared_buffer.h
[modify] https://crrev.com/99ef4c2a857b7e875f3689161493a0e8c508df55/third_party/blink/renderer/platform/shared_buffer_test.cc

We landed a couple of changes to address issue #1 and  #3  in c#13.

@dcasorran I believe either of these should address your problem, and the first one at least should be in Canary by now.  Do you mind verifying when you get a chance?

[Auto-generated comment by a script] We noticed that this issue is targeted for M-68; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-68 label, otherwise remove Merge-TBD label. Thanks.

Hi @fmalita, Great work there, i can confirm the issue is fixed in Canary 69.0.3475.0

Thanks a lot! :)

Thanks for verifying.

Of the two changes, ce42a9d24c7aac91d073851e2ed4fc3180a8384d is the safer merge candidate -- requesting an M68 merge.

This bug requires manual review: M68 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Approving change in #19 for merge to 68. Branch:3440

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/151c1103d059c11b9d4d505ecb3b203908d642bd

commit 151c1103d059c11b9d4d505ecb3b203908d642bd
Author: Florin Malita <fmalita@chromium.org>
Date: Mon Jul 02 20:10:13 2018

Avoid unnecessary SharedBuffer data segmentation

When constructing SharedBuffers from flat data, we currently defer to
SharedBuffer::AppendInternal() -- which splits the source into segments.

This is unnecessary overhead (because SharedBuffer can already track a
single flat slab in buffer_), and can generate even more churn
downstream for clients which require a flattened data view (we're
effectively splitting then reassembling the data source for no reason).

Update SharedBuffer's flat data constructors to store all the data
contiguously in buffer_ (similar to SharedBuffer::AdoptVector).

Bug:  853500 
Change-Id: I523f23859e62c76225c8d5345005c5bd7911d9c3
Reviewed-on: https://chromium-review.googlesource.com/1113661
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#570385}(cherry picked from commit ce42a9d24c7aac91d073851e2ed4fc3180a8384d)
Reviewed-on: https://chromium-review.googlesource.com/1123037
Reviewed-by: Florin Malita <fmalita@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#582}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/151c1103d059c11b9d4d505ecb3b203908d642bd/third_party/blink/renderer/platform/shared_buffer.cc
[modify] https://crrev.com/151c1103d059c11b9d4d505ecb3b203908d642bd/third_party/blink/renderer/platform/shared_buffer_test.cc