[Webkit-unassigned] [Bug 93127] Adding APIs to Chromium WebKit API to allow for creating and monitoring frame hierarchy.

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Nasko Oskov <nasko at chromium.org>  2012-08-03 15:50:10 PST ---
(From update of attachment 156390)
View in context: https://bugs.webkit.org/attachment.cgi?id=156390&action=review

>> Source/WebKit/chromium/public/WebDocument.h:108
>> +    WEBKIT_EXPORT WebElement createElement(const WebString&);
> 
> I'd name this parameter.  Presumable it's the tag name?

Add the name - tagName.

>>> Source/WebKit/chromium/public/WebFrameClient.h:112
>>>      // This frame is about to be closed.
>> 
>> Can you add a comment explaining the lifecycle of a frame w.r.t. these functions?  Presumably didCreateFrame and frameDetached are always called at the beginning and end of the lifecycle, respectively.  From our discussion, it sounds like willClose is called before frameDetached, but only in some situations...
> 
> we need to remove willClose from the API.  it is never what you want.  frameDetached should probably also be renamed at some point to didDetachFrame.

I've added comments to reflect the lifetime. willClose is called very very late, when we have committed the provisional load and closing out old data sources. It wasn't apparent at all from the naming and I'd agree with Darin that this is not something you really want.

>> Source/WebKit/chromium/public/WebFrameClient.h:406
>> +        WebFrame* targetFrame,
> 
> Doesn't this break the API?  Do we need to land this change in stages with a stub?

Yes, you are totally right. My first mixed patch for Chrome and WebKit : ). I've added a stub.

>> Source/WebKit/chromium/src/WebFrameImpl.cpp:594
>> +}
> 
> Can we change these API names to match the WebCore names?  That might make your patch a bit harder to land...

I've added WebFrame::uniqueName. Once the change makes it over to the Chromium tree, I will change all callers to use that function and will subsequently rename the assignedName() to name(). Does that sound like a good plan?

>> Source/WebKit/chromium/src/WebFrameImpl.cpp:2136
>> +      m_client->didCreateFrame(this, webframe.get());
> 
> four-space indent

Fixed.

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