Closed
Bug 1083855
Opened 12 years ago
Closed 12 years ago
Destroy the frames for an element before calling UnbindFromTree and changing its shadowroot/binding in Element::CreateShadowRoot
Categories
(Core :: Layout, defect, P4)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
References
Details
Attachments
(6 files)
|
25.33 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
|
7.19 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
|
9.28 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
|
2.18 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
|
2.29 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
|
17.17 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
No description provided.
| Assignee | ||
Updated•12 years ago
|
Priority: -- → P1
| Assignee | ||
Updated•12 years ago
|
Priority: P1 → P4
| Assignee | ||
Comment 1•12 years ago
|
||
Attachment #8506200 -
Flags: review?(bzbarsky)
| Assignee | ||
Comment 2•12 years ago
|
||
Attachment #8506201 -
Flags: review?(bzbarsky)
| Assignee | ||
Comment 3•12 years ago
|
||
Attachment #8506202 -
Flags: review?(bzbarsky)
| Assignee | ||
Comment 4•12 years ago
|
||
Attachment #8506203 -
Flags: review?(bzbarsky)
| Assignee | ||
Comment 5•12 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 | ||
Comment 6•12 years ago
|
||
Attachment #8506535 -
Flags: review?(bzbarsky)
| Assignee | ||
Comment 7•12 years ago
|
||
With all 5 parts:
https://tbpl.mozilla.org/?tree=Try&rev=3debe57db66b
Comment 8•12 years ago
|
||
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 9•12 years ago
|
||
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 10•12 years ago
|
||
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 11•12 years ago
|
||
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 12•12 years ago
|
||
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 13•12 years ago
|
||
Attachment #8509524 -
Flags: review?(bzbarsky)
| Assignee | ||
Comment 14•12 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 15•12 years ago
|
||
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+
| Assignee | ||
Comment 16•12 years ago
|
||
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
Flags: in-testsuite-
Comment 17•12 years ago
|
||
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: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•