Migrated from  https://crbug.com/842019#c16 :

I feel disabling the streaming cancel in OnPurgeMemory() (i.e. remove ClassicPendingScript::OnPurgeMemory()) is the simplest fix. tasak@, do you expect performance/memory regressions caused by this?

Fixing this so that we can correctly cancel the streaming will be more complicated.
IIUC OnPurgeMemory() is the only code path that cancels the streaming from ClassicPendingScript, either before or after resource load finish.
Also, this is the only caller of ClassicPendingScript::CancelStreaming() that needs finish notification to PendingScriptClient after CancelStreaming(), because other callers are for disposing PendingScript and thus we no longer need any action after CancelStreaming().

I feel we should cleanup/simplify the semantics of script streaming cancellation, as we have similar but different cancel-like methods like:
- ClassicPendingScript::CancelStreaming()
- ScriptStreamer::Cancel()
- SourceStream::Cancel()
- ScriptStreamer::SuppressStreaming()

vogelheim@, WDYT?

I think, the reason why I added ClassicPendingScript::OnPurgeMemory() is that sometimes lots of memory will be used to store downloaded scripts...

However, as far as I investigated, purging discardable memory (e.g. cc) is the most effective. I think, it's ok to disable ClassingPendingScript::OnPurgeMemory().

I just noticed there is at most one script being streamed, and thus OnPurgeMemory() can cancel at most one script's streaming. So the memory gain is at most the memory additionally used for streaming one script, which I feel limited.

Can we please make a change to avoid the incorrect cancelation of streaming here? This may be the cause of random timeouts on our machines such as  Issue 820225 , and I'd like to eliminate this as a possible cause.

Thanks.

I upload a CL to remove ClassicPendingScript::OnPurgeMemory() instead of you guys
 - https://chromium-review.googlesource.com/c/chromium/src/+/1080434

Feel free to give me any comment on it.

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

commit 8304866472d428ea78e8faa88127623e4f415d68
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Fri Jun 01 20:36:49 2018

Avoid canceling streaming in ClassicPendingScript::OnPurgeMemory()

According to the investigation of  Bug 846951 , we suspect that the cancellation of
streaming caused the random timeouts on the mac fyi bots. Additionally, the cancelation
of a script should be done other places and the memory gain done by canceling of streaming
is limited. So this CL removes to call CancelStreaming().

Bug:  846951 
Change-Id: Ibe8ea0bfc0cbfdb3348fb2daad89c0ff8ec08356
Reviewed-on: https://chromium-review.googlesource.com/1080434
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563779}
[modify] https://crrev.com/8304866472d428ea78e8faa88127623e4f415d68/third_party/blink/renderer/core/script/classic_pending_script.cc

Shall we merge the CL to M-68 (and even to stable)?

If this fixes the timeout in WebGL tests ( https://crbug.com/842019#c20 ), then I expect similar timeouts (or never-executed scripts) occur in the wild due to this issue.
Because such timeouts are quite hard to debug (only occurs on OnPurgeMemory()), preventing such timeouts is greatly helpful for stability.

Also, the CL is quite safe to merge, just removing a CancelStreaming() call.

Sure, I think merging back would be a good idea. M-68 sounds fine; I don't think this is being hit often enough in the wild to warrant merging to stable. Requesting merge.

Note: spot-checked a couple of bots on the chromium.gpu.fyi waterfall:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20FYI%20Retina%20Release%20%28AMD%29?limit=200
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20FYI%20Release%20%28Intel%29?limit=200

and not seeing any timeouts of this form since the above CL landed.

Can also check:
https://ci.chromium.org/p/chromium/g/chromium.gpu.fyi/console

and sheriff-o-matic.

Your change meets the bar and is auto-approved for M68. Please go ahead and merge the CL to branch 3440 manually. Please contact 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

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

commit fd027332726fa47c7e09d1ca6f959898b2121666
Author: Gyuyoung Kim <gyuyoung.kim@lge.com>
Date: Tue Jun 05 21:56:04 2018

Avoid canceling streaming in ClassicPendingScript::OnPurgeMemory()

According to the investigation of  Bug 846951 , we suspect that the cancellation of
streaming caused the random timeouts on the mac fyi bots. Additionally, the cancelation
of a script should be done other places and the memory gain done by canceling of streaming
is limited. So this CL removes to call CancelStreaming().

Author: Gyuyoung Kim <gyuyoung.kim@lge.com>

Bug:  846951 
Change-Id: Ibe8ea0bfc0cbfdb3348fb2daad89c0ff8ec08356
Reviewed-on: https://chromium-review.googlesource.com/1080434
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#563779}(cherry picked from commit 8304866472d428ea78e8faa88127623e4f415d68)
Reviewed-on: https://chromium-review.googlesource.com/1087369
Cr-Commit-Position: refs/branch-heads/3440@{#196}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/fd027332726fa47c7e09d1ca6f959898b2121666/third_party/blink/renderer/core/script/classic_pending_script.cc

Should this issue be marked as fixed?

Fixed.

I still expect this occurs (not frequently but also not negligibly rarely) on stable, because it occurs when:
- The memory pressure becomes high (frequent),
- Script streaming is ongoing at that time (frequent), and
- The streamed script is completely loaded (frequent if streaming-after-ready is enabled).
This causes the streamed script not executed and Document's load events not fired.

This kind of bug is very hard to reproduce and triage (because both the behaviors of memory pressure and script streaming are hard to reproduce), so I feel merging is helpful for stability and debuggability.

On the other hand, this is not a recent regression and not an security issue.

Requesting merge to M-67 stable, and defer decisions to M-67 release owners.

As this is a blink issue applying all OSs except iOS.

Based on #10 and #14, rejecting merge to M67 as M67 stable is out since 05/29 and we're ONLY taking absolutely critical merges in at this point. 

+cmasso@ as FYI