Closed
Bug 1241651
Opened 8 years ago
Closed 8 years ago
remove nsPresContext::GetDisplayRootPresContext
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla46
People
(Reporter: tnikkel, Assigned: tnikkel)
Details
Attachments
(3 files)
7.16 KB,
patch
|
mattwoodrow
:
review+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
7.09 KB,
patch
|
Details | Diff | Splinter Review | |
4.86 KB,
patch
|
ritu
:
approval-mozilla-esr38+
|
Details | Diff | Splinter Review |
We have nsPresContext::GetDisplayRootPresContext and nsPresContext::GetRootPresContext. The difference is that GetDisplayRootPresContext looks at the document and asks for it's parent document. This should be unnecessary, and even potentially wrong because if GetParentPresContext() does not find a parent then as far as layout is concerned it does not have a parent document. Because if there is no link via the views that GetParentPresContext looks at then the current prescontext can't be descended into from any parent document via frames or views (ie painting and BuildDisplayList and event targeting). This was added in http://hg.mozilla.org/mozilla-central/rev/79cd5721b7b4 with the comment "Make GetParentPresContext() succeed when the current PresContext has no frames". The GetParentPresContext of that era http://hg.mozilla.org/mozilla-central/file/79cd5721b7b4/layout/base/nsPresContext.cpp#l1170 tried to get the root frame of the document, and failed if it couldn't. However, document's don't always have root frames (for example early in initialization). They do always have views. GetParentPresContext was changed to use views (so it didn't fail when there was no root frame) in http://hg.mozilla.org/mozilla-central/rev/3d20e0577cbf. Thus, the originally reason for the creation of GetDisplayRootPresContext is no more.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8710698 -
Flags: review?(matt.woodrow)
Assignee | ||
Comment 2•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6916089ba8d6
Updated•8 years ago
|
Attachment #8710698 -
Flags: review?(matt.woodrow) → review+
Comment 4•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cc987640612c
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8710698 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: existed for a very long time [User impact if declined]: may fix the crashes of bug 1241217? but we need more data to be sure, uplifted would help with that [Describe test coverage new/current, TreeHerder]: not specifically tested [Risks and why]: low risk [String/UUID change made/needed]: none
Attachment #8710698 -
Flags: approval-mozilla-beta?
Updated•8 years ago
|
status-firefox45:
--- → affected
Comment 6•8 years ago
|
||
Comment on attachment 8710698 [details] [diff] [review] patch Fix a crash, taking it. Should be in 45 beta 2.
Attachment #8710698 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
[:~/mozilla/unified] $ hg graft -er cc987640612c grafting 323187:cc987640612c "Bug 1241651. Remove nsPresContext::GetDisplayRootPresContext. r=mattwoodrow" merging layout/base/nsPresContext.cpp merging layout/base/nsPresContext.h merging layout/base/nsRefreshDriver.cpp merging view/nsView.cpp warning: conflicts while merging view/nsView.cpp! (edit, then use 'hg resolve --mark') abort: unresolved conflicts, can't continue (use hg resolve and hg graft --continue Can we get a rebased patch for beta?
Flags: needinfo?(tnikkel)
Assignee | ||
Comment 8•8 years ago
|
||
Flags: needinfo?(tnikkel)
Assignee | ||
Comment 10•8 years ago
|
||
[Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: this is for bug 1241217 User impact if declined: crashes Fix Landed on Version: on nightly in 46, but it got uplifted Risk to taking this patch (and alternatives if risky): pretty safe String or UUID changes made by this patch: none See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8724945 -
Flags: approval-mozilla-esr38?
status-firefox-esr38:
--- → affected
tracking-firefox-esr38:
--- → 45+
Comment on attachment 8724945 [details] [diff] [review] patch for esr38 Fixes a sec-crit, meets ESR uplift bar.
Attachment #8724945 -
Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Comment 12•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr38/rev/5232087a38c8
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•