[webkit-reviews] review denied: [Bug 46915] Windows client needs updating when live iframe element is moved between pages : [Attachment 71226] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 22 15:03:42 PDT 2010


Adam Roben (aroben) <aroben at apple.com> has denied Jenn Braithwaite
<jennb at chromium.org>'s request for review:
Bug 46915: Windows client needs updating when live iframe element is moved
between pages
https://bugs.webkit.org/show_bug.cgi?id=46915

Attachment 71226: patch
https://bugs.webkit.org/attachment.cgi?id=71226&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=71226&action=review

>>> WebKit/win/WebCoreSupport/WebFrameLoaderClient.cpp:742
>>>  }
>> 
>> Does the passed-in Page correspond to the new document? If so you can just
do:
>> 
>> WebView* newWebView = core(page);
>> 
>> (The name of this function is confusing; it's talking about moving a frame
between pages, but says "document" instead of "page"!)
> 
> The passed in page is the old page.  I will be removing that argument after
bug 44713 is fixed as it turns out that argument is not needed for the fix.

This code will crash if coreFrame doesn't have a parent frame, or that parent
frame doesn't have a WebFrame associated with it. You should either add some
assertions or some null-checks. Other than that this looks fine.


More information about the webkit-reviews mailing list