Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Automatically adding ccs based on suspected regression changelists:

[CI] PaintInvalidationReason::kChunkAppeared and kChunkDisappeared by wangxianzhu@chromium.org - https://chromium.googlesource.com/chromium/src/+/fe5edcbdc288826325a27bcf0c9fdb0948e3764f

Implement process isolation for chrome-error:// documents. by nasko@chromium.org - https://chromium.googlesource.com/chromium/src/+/d83b571dc700010f144b68fe3d8628923c008773

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.

Adding thestig@ for a printing, since according to the stack trace, we are trying to print a frame during detach, which seem like the wrong thing to me:

#8 0x5605dfb5d316 in blink::ChromeClient::Print(blink::LocalFrame*) third_party/blink/renderer/core/page/chrome_client.cc:249:3
#9 0x5605de886a5e in blink::LocalDOMWindow::print(blink::ScriptState*) third_party/blink/renderer/core/frame/local_dom_window.cc:727:27
#10 0x5605dfa9437f in blink::FrameLoader::DidFinishNavigation() third_party/blink/renderer/core/loader/frame_loader.cc:484:26
#11 0x5605dfa49911 in blink::DocumentLoader::StopLoading() third_party/blink/renderer/core/loader/document_loader.cc:807:5
#12 0x5605dfa91f0e in blink::FrameLoader::StopAllLoaders() third_party/blink/renderer/core/loader/frame_loader.cc:1022:23
#13 0x5605de8b8e42 in blink::LocalFrame::Detach(blink::FrameDetachType) third_party/blink/renderer/core/frame/local_frame.cc:353:11

Should we be checking in LocalDOMWindow::print that the frame is not detached? It seems like nothing good can come out trying to print a detached frame.

FYI, this may be a duplicate of  bug 840064 .

 Issue 840064  has been merged into this issue.

Firefox (and similarly Safari) actually prints something. (See attachment.) I'm slightly concerned there are web developers out there who are relying on this kind of behavior.

Deleted: mozilla.pdf 7.8 KB

mozilla.pdf 7.8 KB Download

Are we sure there are devs that rely on this? I would not be surprised at all if this is just a side effect of how browser engines evolved. Given that this was generated by ClusterFuzz, I wouldn't expect by default this to be behavior folks rely on. If folks were to depend on something to print, wouldn't it be better to be in the unload handler? I'm very much inclined to say that if shouldn't be printing a detached frame - there is nothing good that can come out of it.

I hope nobody is depending on this behavior either, but who knows?

Browsers actually don't allow window.print() / window.alert() / etc, during unload anymore. I've seen some pages do this though.

{ window.print(); window.location = foo; // or some other navigation }


Taking a step back, I noticed in the CF case, it tries to insert a copy of itself into an iframe. This ends up triggering 2 print() calls, for both of which the frame being printed is not attached. OTOH, if I change the test case to insert an iframe with "iframe.src = 'about:blank';" then there is only 1 print() call and the frame is attached in this case. Do you know why the attach state is different in these two cases?

Anyway HasSelection() should return false on such situation that
no documents,,,

What would returning false from HasSelection() accomplish? If I read the code and understand it correctly it will just make a decision whether to print the full document or print only the selection. I don't think it will just outright cancel the printing request. Or are we saying that it will avoid calling into ComputeVisibleSelectionInDOMTreeDeprecated? Even if it does, I'm not certain we won't hit some other issue after proceeding further.

Adding dcheng@ who usually has strong opinions about what work is allowed in detached state.

Hmm... szager tried to keep this codepath from triggering with https://chromium-review.googlesource.com/1024912, but perhaps there are other ways to reach it. Basically this is a really strange side effect that can be appear when we're swapping local to remote: we've already run (most of) the JS handlers and told the Document to shutdown; however we haven't yet detached the frame, so we still have to call Frame::Detach() which can trigger surprising work (since it doesn't necessarily expect the Document to already be shutdown).

I guess a hacky workaround would be to synchronously navigate to about:blank (or swappedout:// ? =) to do the Document shutdown... but this would probably have other side effects that would be observable...

This call path is different from the one I fixed.  In this one, the call to DidFinishNavigation comes from DocumentLoader::LoadFailed.  That call appears to be optimized out of the call stack, but it's the only way to get to DidFinishNavigation from DocumentLoader::StopLoading.

Having said that, it is another variant of the bug I tried to fix: we shouldn't be trying to display content that results from a failed load.  A reasonable fix might be to pass a "bool prepareForDisplay = true" argument to DidFinishNavigation, to signify whether the document is going to be displayed.  From LoadFailed(), that argument should be false.

FYI, I had to disable error page isolation due to some renderer kills and a functional bug. It just landed as https://chromium-review.googlesource.com/1050431, so I'd expect this to disappear temporarily until I enable it again. It is trivial to enable locally for testing if needed.

ClusterFuzz has detected this issue as fixed in range 557190:557192.

Detailed report: https://clusterfuzz.com/testcase?key=6396519683719168

Fuzzer: inferno_twister
Job Type: linux_ubsan_vptr_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  blink::FrameSelection::ComputeVisibleSelectionInDOMTreeDeprecated
  blink::WebLocalFrameImpl::HasSelection
  printing::PrintRenderFrameHelper::RequestPrintPreview
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=555986:555990
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=557190:557192

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6396519683719168

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 6396519683719168 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Marking Clusterfuzz-Wrong on the basis of comment #14.

Just for clarity, I have reverted the isolation, so ClusterFuzz is right that it no longer repros using its test case. I do agree this is reachable with other paths, so let's keep it open and fix it.

Lower to Pri-3 since it is caused by unusual HTML.

 Issue 854687  has been merged into this issue.