Bug 66814 - [chromium] Renderer crashes when about:gpucrash is loaded
Summary: [chromium] Renderer crashes when about:gpucrash is loaded
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 66820
  Show dependency treegraph
 
Reported: 2011-08-23 15:41 PDT by Iain Merrick
Modified: 2011-08-23 18:18 PDT (History)
4 users (show)

See Also:


Attachments
Patch (5.78 KB, patch)
2011-08-23 16:04 PDT, Iain Merrick
no flags Details | Formatted Diff | Diff
Patch (5.84 KB, patch)
2011-08-23 17:03 PDT, Iain Merrick
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Iain Merrick 2011-08-23 15:41:24 PDT
about:gpucrash simulates a GPU process crash. The renderer should recover seamlessly.
Comment 1 Iain Merrick 2011-08-23 16:04:46 PDT
Created attachment 104924 [details]
Patch
Comment 2 James Robinson 2011-08-23 16:10:04 PDT
Comment on attachment 104924 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=104924&action=review

> Source/WebKit/chromium/src/WebViewImpl.cpp:2667
> +    // Force ViewHostMsg_DidActivateAcceleratedCompositing to be sent so
> +    // that the browser process can reacquire surfaces.
> +    m_isAcceleratedCompositingActive = false;
> +    setIsAcceleratedCompositingActive(success);

why is this different from the previous logic?

looks like you broke page overlays here, any particular reason to change that logic too?
Comment 3 Iain Merrick 2011-08-23 16:58:44 PDT
Comment on attachment 104924 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=104924&action=review

>> Source/WebKit/chromium/src/WebViewImpl.cpp:2667
>> +    setIsAcceleratedCompositingActive(success);
> 
> why is this different from the previous logic?
> 
> looks like you broke page overlays here, any particular reason to change that logic too?

setRootGraphicsLayer was calling setIsAcceleratedCompositingActive, in addition to the stuff that was causing the crash.

It looked to me like the page overlay update was redundant, because that is called on line 2628 inside setIsAcceleratedCompositingActive, but that only happens if m_layerTreeHost is null. So you're right, it's still needed here -- I'll add it back in.
Comment 4 Iain Merrick 2011-08-23 17:03:47 PDT
Created attachment 104934 [details]
Patch
Comment 5 James Robinson 2011-08-23 17:15:13 PDT
Comment on attachment 104934 [details]
Patch

Ah, I see.  Very subtle.  It seems like we've done something wrong with setIsAcceleratedCompositingActive() if we need it do have side effects for true->true transitions.  Maybe we need a separate notion of 'toggling compositing on/off' vs 'please update yourself due to some compositing change'.  That would be for a different patch, though.
Comment 6 WebKit Review Bot 2011-08-23 18:18:13 PDT
Comment on attachment 104934 [details]
Patch

Clearing flags on attachment: 104934

Committed r93679: <http://trac.webkit.org/changeset/93679>
Comment 7 WebKit Review Bot 2011-08-23 18:18:17 PDT
All reviewed patches have been landed.  Closing bug.