[webkit-reviews] review granted: [Bug 185238] Using image map inside a shadow tree results hits a release assert in DocumentOrderedMap::add : [Attachment 339394] Fixed the ordering

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 185238: Using image map inside a shadow tree results hits a release assert
in DocumentOrderedMap::add
https://bugs.webkit.org/show_bug.cgi?id=185238

Attachment 339394: Fixed the ordering

https://bugs.webkit.org/attachment.cgi?id=339394&action=review




--- Comment #5 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 339394
  --> https://bugs.webkit.org/attachment.cgi?id=339394
Fixed the ordering

View in context: https://bugs.webkit.org/attachment.cgi?id=339394&action=review

> Source/WebCore/dom/Document.cpp:-749
> -void Document::addImageElementByUsemap(const AtomicStringImpl& name,
HTMLImageElement& element)
> -{
> -    return m_imagesByUsemap.add(name, element, *this);
> -}

Is there anything else in Document that should be in TreeScope?

> Source/WebCore/dom/Document.h:-385
> -    void addImageElementByUsemap(const AtomicStringImpl&,
HTMLImageElement&);
> -    void removeImageElementByUsemap(const AtomicStringImpl&,
HTMLImageElement&);
> -    HTMLImageElement* imageElementByUsemap(const AtomicStringImpl&) const;
> -

You forgot to remove the Document::m_imagesByUsemap member.

> Source/WebCore/dom/TreeScope.cpp:59
>  struct SameSizeAsTreeScope {
> -    void* pointers[8];
> +    void* pointers[9];
>  };
>  
>  COMPILE_ASSERT(sizeof(TreeScope) == sizeof(SameSizeAsTreeScope),
treescope_should_stay_small);

Is this assert is even valuable? TreeScope is not a very high volume type in
itself and vast majority of per-scope memory use is elsewhere.

> Source/WebCore/dom/TreeScope.cpp:86
> +    m_elementsByName = nullptr;

Ninja bugfix!