Closed
Bug 1297899
Opened 8 years ago
Closed 8 years ago
stylo: Share append/insert/remove restyle logic with Gecko code
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: bholley, Assigned: heycam)
References
Details
Attachments
(10 files)
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
bholley
:
review+
|
Details |
There's a bunch of stuff that happens in RestyleManager::RestyleFor{Append,Insert,Remove} in order to handle changes to :empty selectors and so forth. This should be straightforward to share, but requires a bit of refactoring that I'm going to skip for now.
Comment 1•8 years ago
|
||
I'm looking into this. I'll see how can I address the atomic setting of the style flags, which is somewhat unfortunate.
Updated•8 years ago
|
Assignee: nobody → ecoal95
Reporter | ||
Comment 2•8 years ago
|
||
Thanks Emilio!
One thing I'd eventually like to do is to get rid of the handles and just have everything use base class pointers. We'd then use macros to have BaseClass::Foo do a type-check and then delegate to either GeckoClass::Foo or ServoClass::Foo. Shared could would live in BaseClass::FooInternal.
No need to do that in this patch, but just be aware of the eventual model when doing refactoring that involves the superclass.
Updated•8 years ago
|
Priority: -- → P2
Assignee | ||
Updated•8 years ago
|
Assignee: emilio+bugs → cam
Status: NEW → ASSIGNED
Flags: needinfo?(cam)
Assignee | ||
Comment 4•8 years ago
|
||
Now that we support the node bits to indicate slow selectors etc., we can basically hoist the restyle logic for content insertion/removal up to the base class without change.
Assignee | ||
Comment 5•8 years ago
|
||
Although we do have some support for the node selector bits, we don't always set them correctly, so dynamic updates involving sibling selectors still don't work properly. But we can fix those after this refactoring.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 16•8 years ago
|
||
Reporter | ||
Comment 17•8 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #5)
> Although we do have some support for the node selector bits, we don't always
> set them correctly, so dynamic updates involving sibling selectors still
> don't work properly. But we can fix those after this refactoring.
Please file any such issues as blockers against bug 1337075.
Reporter | ||
Comment 18•8 years ago
|
||
mozreview-review |
Comment on attachment 8836550 [details]
Bug 1297899 - Part 1: Rename RestyleManager.{h,cpp} to GeckoRestyleManager.{h,cpp}.
https://reviewboard.mozilla.org/r/111948/#review113486
Attachment #8836550 -
Flags: review?(bobbyholley) → review+
Reporter | ||
Comment 19•8 years ago
|
||
mozreview-review |
Comment on attachment 8836551 [details]
Bug 1297899 - Part 2: Rename RestyleManagerBase.{h,cpp} to RestyleManager.{h,cpp}.
https://reviewboard.mozilla.org/r/111950/#review113488
Attachment #8836551 -
Flags: review?(bobbyholley) → review+
Reporter | ||
Comment 20•8 years ago
|
||
mozreview-review |
Comment on attachment 8836552 [details]
Bug 1297899 - Part 3: Rename RestyleManager to GeckoRestyleManager and RestyleManagerBase to RestyleManager.
https://reviewboard.mozilla.org/r/111952/#review113494
rs=me
Attachment #8836552 -
Flags: review?(bobbyholley) → review+
Reporter | ||
Comment 21•8 years ago
|
||
mozreview-review |
Comment on attachment 8836553 [details]
Bug 1297899 - Part 4: Store concrete restyle manager type on RestyleManager.
https://reviewboard.mozilla.org/r/111954/#review113496
Attachment #8836553 -
Flags: review?(bobbyholley) → review+
Reporter | ||
Comment 22•8 years ago
|
||
mozreview-review |
Comment on attachment 8836554 [details]
Bug 1297899 - Part 5: Move refcounting from concrete restyle manager classes up to RestyleManager.
https://reviewboard.mozilla.org/r/111956/#review113498
Attachment #8836554 -
Flags: review?(bobbyholley) → review+
Reporter | ||
Comment 23•8 years ago
|
||
mozreview-review |
Comment on attachment 8836555 [details]
Bug 1297899 - Part 6: Move RestyleManagerHandle functionality into RestyleManager.
https://reviewboard.mozilla.org/r/111958/#review113500
Thank you for doing this cleanup!
Attachment #8836555 -
Flags: review?(bobbyholley) → review+
Reporter | ||
Comment 24•8 years ago
|
||
mozreview-review |
Comment on attachment 8836556 [details]
Bug 1297899 - Part 7: Move PostRestyleEventForLazyConstruction up to RestyleManager.
https://reviewboard.mozilla.org/r/111960/#review113502
Attachment #8836556 -
Flags: review?(bobbyholley) → review+
Reporter | ||
Comment 25•8 years ago
|
||
mozreview-review |
Comment on attachment 8836557 [details]
Bug 1297899 - Part 8: Move Content{Inserted,Appended} up to RestyleManager.
https://reviewboard.mozilla.org/r/111962/#review113504
Attachment #8836557 -
Flags: review?(bobbyholley) → review+
Reporter | ||
Comment 26•8 years ago
|
||
mozreview-review |
Comment on attachment 8836559 [details]
Bug 1297899 - Part 10: Test expectation adjustment.
https://reviewboard.mozilla.org/r/111966/#review113514
Attachment #8836559 -
Flags: review?(bobbyholley) → review+
Reporter | ||
Comment 27•8 years ago
|
||
mozreview-review |
Comment on attachment 8836558 [details]
Bug 1297899 - Part 9: Move RestyleFor{InsertOrChange,Append,EmptyChange} and ContentRemoved up to RestyleManager.
https://reviewboard.mozilla.org/r/111964/#review113512
r=me assuming this is just a simple move.
Attachment #8836558 -
Flags: review?(bobbyholley) → review+
Comment 28•8 years ago
|
||
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f24f346ce8cd
Part 1: Rename RestyleManager.{h,cpp} to GeckoRestyleManager.{h,cpp}. r=bholley
https://hg.mozilla.org/integration/autoland/rev/09d3a31f1160
Part 2: Rename RestyleManagerBase.{h,cpp} to RestyleManager.{h,cpp}. r=bholley
https://hg.mozilla.org/integration/autoland/rev/27ebaadd9a79
Part 3: Rename RestyleManager to GeckoRestyleManager and RestyleManagerBase to RestyleManager. r=bholley
https://hg.mozilla.org/integration/autoland/rev/16eb31796cc6
Part 4: Store concrete restyle manager type on RestyleManager. r=bholley
https://hg.mozilla.org/integration/autoland/rev/92187fa35689
Part 5: Move refcounting from concrete restyle manager classes up to RestyleManager. r=bholley
https://hg.mozilla.org/integration/autoland/rev/5b3ce8ae4965
Part 6: Move RestyleManagerHandle functionality into RestyleManager. r=bholley
https://hg.mozilla.org/integration/autoland/rev/fd140aa9a1f6
Part 7: Move PostRestyleEventForLazyConstruction up to RestyleManager. r=bholley
https://hg.mozilla.org/integration/autoland/rev/767da488b5dd
Part 8: Move Content{Inserted,Appended} up to RestyleManager. r=bholley
https://hg.mozilla.org/integration/autoland/rev/41cd73463ef9
Part 9: Move RestyleFor{InsertOrChange,Append,EmptyChange} and ContentRemoved up to RestyleManager. r=bholley
https://hg.mozilla.org/integration/autoland/rev/6504c3bc8435
Part 10: Test expectation adjustment. r=bholley
Comment 29•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f24f346ce8cd
https://hg.mozilla.org/mozilla-central/rev/09d3a31f1160
https://hg.mozilla.org/mozilla-central/rev/27ebaadd9a79
https://hg.mozilla.org/mozilla-central/rev/16eb31796cc6
https://hg.mozilla.org/mozilla-central/rev/92187fa35689
https://hg.mozilla.org/mozilla-central/rev/5b3ce8ae4965
https://hg.mozilla.org/mozilla-central/rev/fd140aa9a1f6
https://hg.mozilla.org/mozilla-central/rev/767da488b5dd
https://hg.mozilla.org/mozilla-central/rev/41cd73463ef9
https://hg.mozilla.org/mozilla-central/rev/6504c3bc8435
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•