Bug 237660 – Main document is leaking on haaretz.co.il due to lazy image loading

• Component: Page Loading
• Status: RESOLVED FIXED

[Chris Dumez]

Main document is leaking on haaretz.co.il due to lazy image loading.

[Radar WebKit Bug Importer]

<rdar://problem/90035071>

[Chris Dumez]

Created attachment 454248 [details]
Patch

[Chris Dumez]

Created attachment 454273 [details]
Patch

[Chris Dumez]

Created attachment 454362 [details]
Patch

[Chris Dumez]

Created attachment 454368 [details]
Patch

[Chris Dumez]

Created attachment 454381 [details]
Patch

[Chris Dumez]

Created attachment 454387 [details]
Patch

[Chris Dumez]

Created attachment 454394 [details]
Patch

[Geoffrey Garen]

Comment on attachment 454394 [details]
Patch

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

r=me

> Source/WebCore/loader/ImageLoader.cpp:183
> +    auto inUpdateFromElementScope = makeScopeExit([this] {

I'm curious: Are you required to put a scope exit in a local variable?

[Chris Dumez]

(In reply to Geoffrey Garen from comment #9)
> Comment on attachment 454394 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=454394&action=review
> 
> r=me
> 
> > Source/WebCore/loader/ImageLoader.cpp:183
> > +    auto inUpdateFromElementScope = makeScopeExit([this] {
> 
> I'm curious: Are you required to put a scope exit in a local variable?

I don't understand the question, do you mean call makeScopeExit() and then not store the return value in a local variable? If so, definitely not and this is why makeScopeExit() is marked as WARN_UNUSED_RETURN. Something needs to control the lifetime of the "scope".

Also note that I usually would have used a simpler SetForScope<bool> for this. However, it turns out that it didn't work here because m_inUpdateFromElementScope is a bitfield (Apparently it is not valid C++ to have a reference to a bitfield data member).

[EWS]

Committed r291141 (248301@main): <https://commits.webkit.org/248301@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454394 [details].

[Geoffrey Garen]

> I don't understand the question, do you mean call makeScopeExit() and then
> not store the return value in a local variable? If so, definitely not and
> this is why makeScopeExit() is marked as WARN_UNUSED_RETURN. Something needs
> to control the lifetime of the "scope".

Got it.

> Also note that I usually would have used a simpler SetForScope<bool> for
> this. However, it turns out that it didn't work here because
> m_inUpdateFromElementScope is a bitfield (Apparently it is not valid C++ to
> have a reference to a bitfield data member).

Yeah, an unfortunate wart in the language.

[Brent Fulgham]

This fix shipped with Safari 15.5 (all platforms).