[Webkit-unassigned] [Bug 70836] [chromium] add API methods to support renderer swaps

[bugzilla-daemon at webkit.org]

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


Karl Koscher <supersat at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #113578|review-                     |
               Flag|                            |




--- Comment #12 from Karl Koscher <supersat at chromium.org>  2011-11-16 15:25:26 PST ---
(From update of attachment 113578)
View in context: https://bugs.webkit.org/attachment.cgi?id=113578&action=review

We decided to implement swapping out this way because it lets us get rid of the renderer-side IPC message filtering. If we were to just send back an blank cross-site response, we'd still need to filter out all of the navigation messages the renderer would send.

>> Source/WebKit/chromium/public/WebDocument.h:75
>> +    WEBKIT_EXPORT static WebDocument create(const WebString& MIMEType,
> 
> nit: mimeType <-- webkit style

Fixed.

>> Source/WebKit/chromium/public/WebDocument.h:76
>> +                                            const WebFrame*,
> 
> nit: I'd probably list the WebFrame parameter first since it is the container for the WebDocument.

Fixed.

>> Source/WebKit/chromium/public/WebDocument.h:77
>> +                                            const WebURL&,
> 
> By passing a WebURL here, it seems like it is possible for the WebFrame and the WebDocument to have different ideas about what URL is loaded in the frame.  Is that really helpful?  It seems like it could cause subtle bugs.  Another idea would be to infer the URL of the document from the frame.

Where does the WebFrame keep the current URL? I couldn't find anywhere that it does...

>> Source/WebKit/chromium/public/WebDocument.h:78
>> +                                            bool inViewSourceMode);
> 
> nit: is it strictly necessary to pass the inViewSourceMode parameter here?  can't you just use WebFrame::enableViewSourceMode(true)?

Fixed.

>> Source/WebKit/chromium/src/WebDocument.cpp:70
>> +        MIMEType, frame ? static_cast<const WebFrameImpl*>(frame)->frame() : 0,
> 
> Do you have a use case for frame being null here?  Maybe we should require that it is non-null.

No, I don't. Fixed.

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