The default bug view has changed. See this FAQ.

Stop touching the frame tree to determine whether a node is editable or not

RESOLVED FIXED in mozilla10

Status

()

Core
Editor
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks: 1 bug)

Trunk
mozilla10
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
nsEditor::IsEditable examines the frame tree to figure out whether an element is editable or not.  We should stop doing that, if we ever want to get lazy frame construction for editable nodes.
(Assignee)

Comment 1

6 years ago
Created attachment 562087 [details] [diff] [review]
WIP

This is the WIP patch that I have.  I'm posting it to the try server to see what it thinks about the patch.  Will ask for review when I get a sense of how good the patch is.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED

Comment 2

6 years ago
Try run for e42789a3cf71 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=e42789a3cf71
Results (out of 171 total builds):
    exception: 2
    success: 155
    warnings: 13
    failure: 1
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-e42789a3cf71

Comment 3

6 years ago
Try run for 58064ab34bf3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=58064ab34bf3
Results (out of 193 total builds):
    success: 156
    warnings: 19
    failure: 18
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-58064ab34bf3
(Assignee)

Updated

6 years ago
Depends on: 695364
(Assignee)

Comment 4

6 years ago
Created attachment 567777 [details] [diff] [review]
Patch (v1)

OK, this is now ready.  The only issue with this is bug 695364, and I think we can ignore that for now.

I also fixed the invalid Android failure assertion on that crashtest, since the test does not assert on Android.
Attachment #562087 - Attachment is obsolete: true
Attachment #567777 - Flags: review?(roc)
Attachment #567777 - Flags: review?(roc) → review+
How much of the win of lazy frame construction does resolving these style contexts take away?
(Assignee)

Comment 6

6 years ago
https://bugzilla.mozilla.org/attachment.cgi?id=567777

(In reply to Timothy Nikkel (:tn) from comment #5)
> How much of the win of lazy frame construction does resolving these style
> contexts take away?

You mean the perf win?  Well, resolving stye contexts is part of what we do for creating frames.  And this only happens when IsEditable is called, which is mostly only when an editing operation happens.

Do you have better ideas on what we can do here?
Target Milestone: --- → mozilla10

Comment 7

6 years ago
Try run for 71527bc03a6e is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=71527bc03a6e
Results (out of 199 total builds):
    success: 189
    warnings: 10
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-71527bc03a6e
https://hg.mozilla.org/mozilla-central/rev/7ec3daabbf47
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(In reply to Ehsan Akhgari [:ehsan] from comment #6)
> Do you have better ideas on what we can do here?

Nope.

Updated

5 years ago
Depends on: 725069
(Assignee)

Updated

5 years ago
Depends on: 796839
You need to log in before you can comment on or make changes to this bug.