[Webkit-unassigned] [Bug 20263] [XBL] Add loading code for XBLBinding

[bugzilla-daemon at webkit.org]

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





------- Comment #3 from jchaffraix at webkit.org  2008-08-08 10:29 PDT -------
(In reply to comment #2)
> (From update of attachment 22619 [edit])
> What does "hasIndex" mean?  Maybe you mean "has fragment"?  #foo is a "fragment
> url", maybe the XBL spec calls it an index?

It was the name used in the legacy code. The spec does not give any specific
term so we will go for fragment url.

> I wonder if it's safe to just grab weak pointers to other documents:
> +    Document* bindingDocument;
> +    if (documentURL == boundElement->document()->url())
> +        // This is the case of an <xbl> element inside another document.
> +        // The binding document is the bound document.
> +        bindingDocument = boundElement->document();

I think it is ok here:
- if it is the bound document, then we hold it with the bound element.
- if it is not, we hold a pointer to the associated CachedResource.

> Does this automatically ref?
> +        m_cachedDocument =
> boundElement->document()->docLoader()->requestXBLDocument(documentURL);
> if so, when is the XBLBinding releasing the CachedDocument?

The Cache code does not use refcounting. Instead they just use raw pointers and
inherits from CachedResourceClient. I will change that.

> How does the .get() make sense here?
> 
> bindingDocument = m_cachedDocument->document().get();
> 
> does document() return a const RefPtr&?

It returns a PassRefPtr. I have thought about that and we need to have a strong
reference to the binding document in case it was loaded because of a binding so
the get() will be removed.

> I'm not sure this check is really necessary:
> if (!m_id.isNull()) {
> 
> the hash-lookup of a NULL pointer won't be many more cycles :)


Indeed.

> Why is it safe for XBLBinding to have a weak pointer to an XBLBindingElement?
> +        Element* bindingElement = bindingDocument->getElementById(m_id);
> +        if (bindingElement->isXBLElement() &&
> static_cast<XBLElement*>(bindingElement)->isXBLBindingElement())
> +            m_bindingElement =
> static_cast<XBLBindingElement*>(bindingElement);
> 
> What if that binding element was removed from the document?

I will change to a RefPtr as it is only safe for the bound element (which has
to clean its bindings when deleted).

> This could probably have a slightly more descriptive name:
> +        String m_id;

Will change to m_bindingID.


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