Bug 253417 - Remove extra call to 'layout' in Document::setParsing
Summary: Remove extra call to 'layout' in Document::setParsing
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-03-05 15:02 PST by Ahmad Saleem
Modified: 2023-03-12 16:03 PDT (History)
5 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2023-03-05 15:02:50 PST 
Hi Team,

While going through Blink's commit - I came across following, where we also have FIXME to rename function of similar:

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/56ef94099e79af642ee0ef5a3c8a3523c1ecc7dc

WebKit Source - https://searchfox.org/wubkat/source/Source/WebCore/dom/Document.h#2319 etc.

^ There is FIXME as well in WebKit Source.

Do we need to merge this or come-up with better name?

Thanks!
Comment 1 zalan 2023-03-06 07:10:16 PST 
It looks like at this point this merge would only including the renaming part (WebKit does not schedule layout inside Document::setParsing)
Comment 2 Ahmad Saleem 2023-03-06 07:33:45 PST 
(In reply to zalan from comment #1)
> It looks like at this point this merge would only including the renaming
> part (WebKit does not schedule layout inside Document::setParsing)

fireLayoutRelatedMilestonesIfNeeded() <- function, does it cause 'relayout'? Just from reading the code, it seems to be "Milestone" related something.
Comment 3 zalan 2023-03-06 07:35:44 PST 
(In reply to Ahmad Saleem from comment #2)
> (In reply to zalan from comment #1)
> > It looks like at this point this merge would only including the renaming
> > part (WebKit does not schedule layout inside Document::setParsing)
> 
> fireLayoutRelatedMilestonesIfNeeded() <- function, does it cause 'relayout'?
No, it is not supposed to.
Comment 4 Ahmad Saleem 2023-03-06 07:36:53 PST 
(In reply to zalan from comment #3)
> (In reply to Ahmad Saleem from comment #2)
> > (In reply to zalan from comment #1)
> > > It looks like at this point this merge would only including the renaming
> > > part (WebKit does not schedule layout inside Document::setParsing)
> > 
> > fireLayoutRelatedMilestonesIfNeeded() <- function, does it cause 'relayout'?
> No, it is not supposed to.

Should I do PR for renaming purposes and get rid of FIXME, at least one less FIXME. As long as you are OK with 'm_isParsing' name instead of 'm_bParsing'?
Comment 5 zalan 2023-03-06 07:37:50 PST 
sounds good!
Comment 6 Radar WebKit Bug Importer 2023-03-12 16:03:16 PDT 
<rdar://problem/106621886>