Bug 208918 – FrameLoader should own its FrameLoaderClient

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[youenn fablet]

FrameLoader should own its FrameLoaderClient

[youenn fablet]

Created attachment 393238 [details]
Patch

[youenn fablet]

Created attachment 393254 [details]
Patch

[youenn fablet]

Comment on attachment 393254 [details]
Patch

Need to fix win port. Putting under review in the meantime.

[youenn fablet]

Created attachment 393353 [details]
Patch

[youenn fablet]

Created attachment 393358 [details]
Patch

[youenn fablet]

Created attachment 393363 [details]
Patch

[Geoffrey Garen]

Comment on attachment 393363 [details]
Patch

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

r=me

> Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:-175
> -void WebFrameLoaderClient::frameLoaderDestroyed()
> -{
> -    m_frame->invalidate();
> -
> -    // Balances explicit ref() in WebFrame::create().
> -    m_frame->deref();
> -}

It is a joy to see code like this disappear 🥳

[WebKit Commit Bot]

Comment on attachment 393363 [details]
Patch

Clearing flags on attachment: 393363

Committed r258339: <https://trac.webkit.org/changeset/258339>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/60377728>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 209179

[youenn fablet]

Created attachment 393840 [details]
Patch for landing

[youenn fablet]

(In reply to youenn fablet from comment #12)
> Created attachment 393840 [details]
> Patch for landing

Fixed rendering issue by delaying setting WebPage::m_mainFrame to exactly what was done before. The issue was that we were unfreezing too quickly in case of ProcessSwapping.
This does not fix the potential raciness in the two DrawingArea messages of the being-swapped WebPageProxy, but this patch does not make it worse.

[youenn fablet]

Created attachment 393842 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 393842 [details]
Patch for landing

Clearing flags on attachment: 393842

Committed r258628: <https://trac.webkit.org/changeset/258628>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Geoffrey Garen]

Can you file a follow up bug about the DrawingAreaProxy race — probably for Chris or Tim? Sounds like something worth fixing.

[youenn fablet]

(In reply to Geoffrey Garen from comment #17)
> Can you file a follow up bug about the DrawingAreaProxy race — probably for
> Chris or Tim? Sounds like something worth fixing.

Sure.
To be clear, the race is hopefully probably theoretical only.
My main issue is that the design seems a bit weak since we keep two IPC listeners from two different processes but for the same page.

I'll do some follow-up clean-up related to WebPage.

[youenn fablet]

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