[Webkit-unassigned] [Bug 96114] [chromium] Make prioritized texture manager not touch backings array on the main thread

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=96114





--- Comment #15 from Adrienne Walker <enne at google.com>  2012-09-10 10:41:35 PST ---
(In reply to comment #12)
> Thanks!
> 
> (In reply to comment #11)
> > I don't follow this.  assertInvariants is for making sure that the sorting is correct.  It seems like you should either only assert when it's sorted or sort every time.  Why do you need to sometimes sort here?
> 
> This is idiosyncratic.  I wanted to add code to test the sorting algorithm, and code to verify that things are sorted at various points in the execution.  The flag "forceResort" is only used in a test which verifies the sorting scheme.  The alternative (which may be better) would be to have make prioritizeBackings public, and have the test explicitly call that (which then implicitly verifies the order in assertInvariants).  Would that be more straightforward.  The only downside of exposing a (basically) internal function just for testing.

I think if you want to test the sorting algorithm, then you should write some tests that test whatever cases you're worried about explicitly.

I think the other thing that worries me is assertInvariants(true) modifies state, so debug builds now have different behavior than release builds.  So, if there's some point where we need to have sorted backings but haven't, then assertInvariants(true) might mask that.

It's also just hard to look at these assertInvariants(true) and assertInvariants(false) from a code reading perspective and have any easy understanding of what value needs to be passed when.

I like the patch other than that bit.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.