Bug 73939 - [chromium] Don't crash if tile upload happens without painting first
Summary: [chromium] Don't crash if tile upload happens without painting first
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: Adrienne Walker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-06 11:47 PST by Adrienne Walker
Modified: 2011-12-06 14:26 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.92 KB, patch)
2011-12-06 11:51 PST, Adrienne Walker
no flags Details | Formatted Diff | Diff
Fix ImageLayerChromium to not do this (2.69 KB, patch)
2011-12-06 12:27 PST, Adrienne Walker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrienne Walker 2011-12-06 11:47:14 PST
[chromium] Don't crash if tile upload happens without painting first
Comment 1 Adrienne Walker 2011-12-06 11:51:01 PST
Created attachment 118084 [details]
Patch
Comment 2 Adrienne Walker 2011-12-06 11:56:14 PST
See: http://code.google.com/p/chromium/issues/detail?id=105569

I think this crash is caused by a paint/upload mismatch.  A layer isn't painted but is uploaded from, so its tiler isn't created.  I'm not totally sure where this is happening, but that seems like a likely culprit.

The proper fix is the ForEachCompositorResource functor iteration from https://bugs.webkit.org/show_bug.cgi?id=72752, but I want something small that can be backported to m17.
Comment 3 David Reveman 2011-12-06 12:09:01 PST
Looks good to me. Any idea how we end up calling updateCompositorResources() without prepareToUpdate first?
Comment 4 Adrienne Walker 2011-12-06 12:16:32 PST
(In reply to comment #3)
> Looks good to me. Any idea how we end up calling updateCompositorResources() without prepareToUpdate first?

I'm not totally sure, but there's two totally different code paths, so it seemed really plausible.

Actually, ImageLayerChromium::paintContentsIfDirty has an early out before prepareToUpdate if the visible rect is empty.  That would do it.
Comment 5 Adrienne Walker 2011-12-06 12:27:44 PST
Created attachment 118090 [details]
Fix ImageLayerChromium to not do this
Comment 6 James Robinson 2011-12-06 12:31:08 PST
Comment on attachment 118090 [details]
Fix ImageLayerChromium to not do this

This looks safe
Comment 7 WebKit Review Bot 2011-12-06 14:26:44 PST
Comment on attachment 118090 [details]
Fix ImageLayerChromium to not do this

Clearing flags on attachment: 118090

Committed r102180: <http://trac.webkit.org/changeset/102180>
Comment 8 WebKit Review Bot 2011-12-06 14:26:49 PST
All reviewed patches have been landed.  Closing bug.