Closed Bug 1314520 Opened 3 years ago Closed 3 years ago

Assertion failed in TabParent::Show when the GPU process aborts

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: dvander, Assigned: dvander)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Pushed by danderson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cfa59e10e83
Ensure that CompositorBridgeChild retains an IPDL ref while owning a Transport. (bug 1314520, r=mattwoodrow)
https://hg.mozilla.org/mozilla-central/rev/6cfa59e10e83
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Whoops, that was the wrong bug number - should have been bug 1315886.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This one is a bit insidious. While it's fine for most of this function to fail, it exposes another problem. The compositor pipe we send from the GPU process to the content process can terminate before the content process opens it. In this case, the content process fails to open the pipe, but still constructs a MessageChannel around a broken Transport.

This MessageChannel assumes the Transport is working and blocks forever sending any sync message.
Assignee: nobody → dvander
Status: REOPENED → ASSIGNED
Attached patch part 1, fix ProcessingError (obsolete) — Splinter Review
We check IPCOpen here, but the actual failure can occur in the SendPAPZConstructor. This may only be due to a separate bug that I'm fixing in part 3, but this is a cleaner check anyway.
Attachment #8809234 - Flags: review?(rhunt)
whoops wrong patch
Attachment #8809234 - Attachment is obsolete: true
Attachment #8809234 - Flags: review?(rhunt)
Attachment #8809235 - Flags: review?(rhunt)
TabChild::InitRenderingState can fail if the GPU process has died while the tab loads. Unfortunately, this currently causes TabChild to completely stop a bunch of logic that hooks it up to the rendering stack.

This patch makes the failure checks more precise, so we can continue running code like the TabId:LayersId association and first-paint observer.
Attachment #8809238 - Flags: review?(wmccloskey)
Comment on attachment 8809235 [details] [diff] [review]
part 1, fix ProcessingError

> 
>   APZChild* apz = ContentProcessController::Create(aTabId);
>-  return CompositorBridgeChild::Get()->SendPAPZConstructor(apz, aLayersId);
>+  if (!CompositorBridgeChild::Get()->SendPAPZConstructor(apz, aLayersId)) {
>+    return true;
>+  }
>+
>+  return true;
> }
> 

Could we use 'Unused << ...' here and get rid of the conditional and just return true?

Otherwise looks good to me.
Attachment #8809235 - Flags: review?(rhunt) → review+
Comment on attachment 8809238 [details] [diff] [review]
part 2, fix TabChild::RecvShow

Review of attachment 8809238 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/TabChild.cpp
@@ +2579,5 @@
>      // compositor context.
>      PCompositorBridgeChild* compositorChild = CompositorBridgeChild::Get();
>      if (!compositorChild) {
>        NS_WARNING("failed to get CompositorBridgeChild instance");
>        PRenderFrameChild::Send__delete__(remoteFrame);

It seems like this call should be removed, as well as others like it.
Attachment #8809238 - Flags: review?(wmccloskey) → review+
Pushed by danderson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47fc69e9d830
Don't trigger a ProcessingError when we fail to send a PAPZConstructor. (bug 1314520 part 1, r=rhunt)
https://hg.mozilla.org/integration/mozilla-inbound/rev/313d7a1c1bbc
Finish initializing TabChild rendering state even if compositing IPC is lost. (bug 1314520 part 2, r=billm)
https://hg.mozilla.org/mozilla-central/rev/47fc69e9d830
https://hg.mozilla.org/mozilla-central/rev/313d7a1c1bbc
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.