Destroy the frames for an element before calling UnbindFromTree and changing its shadowroot/binding in Element::CreateShadowRoot

RESOLVED FIXED in mozilla36

Status

()

defect
P4
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mats, Assigned: mats)

Tracking

unspecified
mozilla36
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments)

Assignee

Description

5 years ago
No description provided.
Assignee

Updated

5 years ago
Priority: -- → P1
Assignee

Updated

5 years ago
Priority: P1 → P4
Assignee

Comment 5

5 years ago
FYI, I'm working on a part 5 for nsXBLBindingRequest::DocumentLoaded (similar to part 4),
but I figured you could start reviewing the above.
Assignee

Updated

5 years ago
Blocks: 907396
Comment on attachment 8506200 [details] [diff] [review]
part 1, Add an aDestroyedFramesFor out param to ContentRemoved and propagate it (and aFlags) to RecreateFramesForContent etc. (idempotent change)

r=me, but can we remove the optionality of the RecreateFramesForContent arguments later in this patch series, please?  Don't want us to miss callsites that need updating.
Attachment #8506200 - Flags: review?(bzbarsky) → review+
Comment on attachment 8506201 [details] [diff] [review]
part 2, Add a REMOVE_DESTROY_FRAMES value to RemoveFlags.  Make RecreateFramesForContent skip recreating the frames when aFlags has that value.

r=me
Attachment #8506201 - Flags: review?(bzbarsky) → review+
Comment on attachment 8506202 [details] [diff] [review]
part 3, Implement nsIPresShell::DestroyFramesFor/CreateFramesFor.

Why not allow lazy construction in CreateFramesFor?  And if we do, perhaps call it MaybeCreateFramesFor or something?

r=me
Attachment #8506202 - Flags: review?(bzbarsky) → review+
Comment on attachment 8506203 [details] [diff] [review]
part 4, Make CreateShadowRoot first call DestroyFramesFor, then setup the new shadow root, then CreateFramesFor.

r=me
Attachment #8506203 - Flags: review?(bzbarsky) → review+
Comment on attachment 8506535 [details] [diff] [review]
part 5, Make nsXBLBindingRequest::DocumentLoaded first call DestroyFramesFor, then load the binding, then CreateFramesFor.

r=me
Attachment #8506535 - Flags: review?(bzbarsky) → review+
Assignee

Comment 14

5 years ago
(In reply to Boris Zbarsky [:bz] from comment #10)
> Why not allow lazy construction in CreateFramesFor?

Because that's not what we currently do and I want to stay as close
as possible to the current behavior:
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp?rev=968aa79b1200#8984

ContentRangeInserted discards the passed in frame state when lazy
construction happens, AFAICT.
Comment on attachment 8509524 [details] [diff] [review]
part 6, Make RecreateFramesForContent params non-optional.

> ContentRangeInserted discards the passed in frame state when lazy
> construction happens, AFAICT.

Ah, ok.

Thanks for doing this part.  r=me
Attachment #8509524 - Flags: review?(bzbarsky) → review+
You need to log in before you can comment on or make changes to this bug.