Closed Bug 1083855 Opened 6 years ago Closed 6 years ago
Destroy the frames for an element before calling Unbind
From Tree and changing its shadowroot/binding in Element::Create Shadow Root
No description provided.
Attachment #8506203 - Flags: review?(bzbarsky)
FYI, I'm working on a part 5 for nsXBLBindingRequest::DocumentLoaded (similar to part 4), but I figured you could start reviewing the above.
With all 5 parts: https://tbpl.mozilla.org/?tree=Try&rev=3debe57db66b
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+
(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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d33a443b57ef https://hg.mozilla.org/integration/mozilla-inbound/rev/fb5ee1150bef https://hg.mozilla.org/integration/mozilla-inbound/rev/227f39d07217 https://hg.mozilla.org/integration/mozilla-inbound/rev/ae0d803ec13e https://hg.mozilla.org/integration/mozilla-inbound/rev/73854805310b https://hg.mozilla.org/integration/mozilla-inbound/rev/2c56d741c3fd
https://hg.mozilla.org/mozilla-central/rev/d33a443b57ef https://hg.mozilla.org/mozilla-central/rev/fb5ee1150bef https://hg.mozilla.org/mozilla-central/rev/227f39d07217 https://hg.mozilla.org/mozilla-central/rev/ae0d803ec13e https://hg.mozilla.org/mozilla-central/rev/73854805310b https://hg.mozilla.org/mozilla-central/rev/2c56d741c3fd
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.