[Webkit-unassigned] [Bug 104472] [Windows]ASSERTION failed in Windows: css3/css3-modsel-33.html

[bugzilla-daemon at webkit.org]

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


Brent Fulgham <bfulgham at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #178402|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #5 from Brent Fulgham <bfulgham at webkit.org>  2012-12-10 11:40:34 PST ---
(From update of attachment 178402)
View in context: https://bugs.webkit.org/attachment.cgi?id=178402&action=review

Thanks for taking the time to post this fix.  I think it looks good, but we would like you to move the COMPtr declaration inside the two "if" cases. This will make it clearer that the COMPtr is not needed elsewhere.
For this reason, r-, but please update the patch and I'll approve it.

>>> Source/WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp:429
>>> +    frameLoadDelegatePrivate = 0;
>> 
>> This seems like a good change, but would it be clearer if we used separate COMPtrs for the two possible cases? Maybe we should have each one declared within the scope of the DidFirstlayout and DidFirstVisuallyNonEmpty Layout?
> 
> I agree, Brent's plan (two separate locals inside the if blocks) is clearer.

Okay -- please move the scope of "COMPtr<IWebFrameLoadDelegatePrivate> frameLoadDelegatePrivate" to be enclosed inside the two cases.

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