1630704 - Merge nsFrame into nsIFrame

Status: RESOLVED FIXED

(Core :: Layout, task, P3)

Description

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

nsFrame and nsIFrame are effectively same, so we want to merge them.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

What are the set of methods that would become public methods on nsIFrame if we do this? Should they be marked as layout-internal in some way? (Whether they should be probably depends on what they are...)

Comment 2

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Merge nsBox into nsIFrame

We want to merge nsFrame into nsIFrame, so this patch merges nsBox first which is the superclass of nsFrame.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6bb1b6d0cde8
Merge nsBox into nsIFrame r=emilio

Comment 4

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/6bb1b6d0cde8

Comment 5

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Remove NS_FRAME_TRACE_MSG

NS_FRAME_TRACE_MSG is identical to NS_FRAME_TRACE, no need to keep it.

Comment 6

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Move nsBidi* types to nsBidi.h

nsBidi.h currently includes nsIFrame.h only to import nsBidi* types. This patch moves those types to nsBidi.h.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1a7a3f86d10
Remove NS_FRAME_TRACE_MSG r=emilio

Comment 8

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/e1a7a3f86d10

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f4da5c776b0
Move nsBidi* types to nsBidi.h r=emilio

Comment 10

[Razvan Maries]

Backed out for build bustages.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=299398428&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=8f4da5c776b052c6dba8dcd86742521d7de0e732

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299398428&repo=autoland&lineNumber=24891

Backout: https://hg.mozilla.org/integration/autoland/rev/0f229348ee97584233d5b125a19942cee52a7ffb

Comment 11

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

It doesn't seem to be relevant here, EventDispatcher.cpp doesn't include nsBidi nor nsIFrame.

Comment 12

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Merge standalone overrriding methods to nsIFrame

This patch merges nsFrame methods to nsIFrame, where merging only requires simple signature changes.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39acb94eae20
Merge standalone overrriding methods to nsIFrame r=emilio

Comment 14

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/39acb94eae20

Comment 15

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Merge overridding nsFrame methods to nsIFrame

This follows D73070. Remainings are Init(), DestroyFrom(), and several functions that are not used by nsIFrame methods, which will be also merged in separate patch(s).

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/599181b1842e
Merge overridding nsFrame methods to nsIFrame r=emilio

Comment 17

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/599181b1842e

Comment 18

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Merge XUL methods to nsIFrame

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ba46972ba72
Merge XUL methods to nsIFrame r=emilio

Comment 20

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/3ba46972ba72

Comment 21

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 6: Move helper methods to nsContainerFrame

Those methods are only used in subclasses of nsContainerFrame.

Comment 22

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 7: Move layout debugging methods to nsIFrame

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e32b21634a0c
Part 6: Move helper methods to nsContainerFrame r=emilio

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b46694bafa8a
Part 7: Move layout debugging methods to nsIFrame r=emilio

Comment 25

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/e32b21634a0c
https://hg.mozilla.org/mozilla-central/rev/b46694bafa8a

Comment 26

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Is there anything left to do in this bug? Should we close this?

Comment 27

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Some functions, mainly debugging only ones are remaining. Those should be moved before closing this.

Comment 28

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 8: Move FireDOMEvent()

Comment 29

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 9: Move GetFrameName()

Depends on D79205

Comment 30

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 10: Move ShowFrameBorders() and friends

Depends on D79206

Comment 31

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 11: Move ShouldApplyOverflowClipping()

Depends on D79207

Comment 32

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 12: Move GetLineNumber()

Depends on D79208

Comment 33

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 13: Move GetFrameId() and macros

Depends on D79209

Comment 34

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 14: Move GetFirst/LastLeaf()

Depends on D79210

Comment 35

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 15: Move GetNextPrevLineFromeBlockFrame()

Depends on D79211

Comment 36

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 16: Move CalcContentOffsetsFromFramePoint()

Depends on D79212

Comment 37

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 17: Move PeekBackwardAndForward()

Depends on D79213

Comment 38

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 18: Move FinishReflowWithAbsoluteFrames()

Depends on D79214

Comment 39

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 19: Move CreateView()

Depends on D79215

Comment 40

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 20: Move CorrectStyleParentFrame()

Depends on D79216

Comment 41

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 21: Move debugging trace/display methods

Depends on D79217

Comment 42

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 22: Move DidReflow()

Depends on D79218

Comment 43

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 23: Move Display methods

Depends on D79219

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1caef46ab4cc
Part 8: Move FireDOMEvent() r=emilio
https://hg.mozilla.org/integration/autoland/rev/2366fa8dc5fd
Part 9: Move GetFrameName() r=emilio
https://hg.mozilla.org/integration/autoland/rev/3c593e12fbd2
Part 10: Move ShowFrameBorders() and friends r=emilio
https://hg.mozilla.org/integration/autoland/rev/c03feb270548
Part 11: Move ShouldApplyOverflowClipping() r=emilio
https://hg.mozilla.org/integration/autoland/rev/f4f14bc9ba47
Part 12: Move GetLineNumber() r=emilio

Comment 45

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1caef46ab4cc
https://hg.mozilla.org/mozilla-central/rev/2366fa8dc5fd
https://hg.mozilla.org/mozilla-central/rev/3c593e12fbd2
https://hg.mozilla.org/mozilla-central/rev/c03feb270548
https://hg.mozilla.org/mozilla-central/rev/f4f14bc9ba47

Comment 46

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb454367513b
Part 13: Move GetFrameId() and macros r=emilio
https://hg.mozilla.org/integration/autoland/rev/7716518d4f61
Part 14: Move GetFirst/LastLeaf() r=emilio
https://hg.mozilla.org/integration/autoland/rev/dfcc82f0f389
Part 15: Move GetNextPrevLineFromeBlockFrame() r=emilio
https://hg.mozilla.org/integration/autoland/rev/07666f80b7f9
Part 16: Move CalcContentOffsetsFromFramePoint() r=emilio
https://hg.mozilla.org/integration/autoland/rev/968ebb05635e
Part 17: Move PeekBackwardAndForward() r=emilio
https://hg.mozilla.org/integration/autoland/rev/b9685b190cef
Part 18: Move FinishReflowWithAbsoluteFrames() r=emilio
https://hg.mozilla.org/integration/autoland/rev/a26000df3ded
Part 19: Move CreateView() r=emilio
https://hg.mozilla.org/integration/autoland/rev/99bdf3e3e02a
Part 20: Move CorrectStyleParentFrame() r=emilio
https://hg.mozilla.org/integration/autoland/rev/96f162d88448
Part 21: Move debugging trace/display methods r=emilio
https://hg.mozilla.org/integration/autoland/rev/0251d93c3af1
Part 22: Move DidReflow() r=emilio
https://hg.mozilla.org/integration/autoland/rev/5d78b02cb493
Part 23: Move Display methods r=emilio

Comment 47

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/eb454367513b
https://hg.mozilla.org/mozilla-central/rev/7716518d4f61
https://hg.mozilla.org/mozilla-central/rev/dfcc82f0f389
https://hg.mozilla.org/mozilla-central/rev/07666f80b7f9
https://hg.mozilla.org/mozilla-central/rev/968ebb05635e
https://hg.mozilla.org/mozilla-central/rev/b9685b190cef
https://hg.mozilla.org/mozilla-central/rev/a26000df3ded
https://hg.mozilla.org/mozilla-central/rev/99bdf3e3e02a
https://hg.mozilla.org/mozilla-central/rev/96f162d88448
https://hg.mozilla.org/mozilla-central/rev/0251d93c3af1
https://hg.mozilla.org/mozilla-central/rev/5d78b02cb493

Comment 48

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Move Init/DestroyFrom to nsIFrame r=emilio

Comment 49

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/631e19f986e4
Move Init/DestroyFrom to nsIFrame r=emilio

Comment 50

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/631e19f986e4

Comment 51

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 25: Construct nsIFrame from NS_NewEmptyFrame r=emilio

Comment 52

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 26: Move ReflowInput import to nsIFrame.h r=emilio

Depends on D81844

Comment 53

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 27: Remove nsFrame from inheritance chain r=emilio

Depends on D81845

Comment 54

[Kagami Rosylight [:saschanaz] (they/them) (OOO until Dec 11)]

Attachment: Bug 1630704 - Part 28: Remove nsFrame r=emilio

Depends on D81864

Comment 55

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9497f465b69b
Part 25: Construct nsIFrame from NS_NewEmptyFrame r=emilio
https://hg.mozilla.org/integration/autoland/rev/d30b9ebb50b3
Part 26: Move ReflowInput import to nsIFrame.h r=emilio
https://hg.mozilla.org/integration/autoland/rev/529c7022e64b
Part 27: Remove nsFrame from inheritance chain r=emilio
https://hg.mozilla.org/integration/autoland/rev/73880713f199
Part 28: Remove nsFrame r=emilio

Comment 56

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/9497f465b69b
https://hg.mozilla.org/mozilla-central/rev/d30b9ebb50b3
https://hg.mozilla.org/mozilla-central/rev/529c7022e64b
https://hg.mozilla.org/mozilla-central/rev/73880713f199