Last Comment Bug 688789 - Stop touching the frame tree to determine whether a node is editable or not
: Stop touching the frame tree to determine whether a node is editable or not
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla10
Assigned To: :Ehsan Akhgari
:
Mentors:
Depends on: 695364 725069 796839
Blocks: 687981
  Show dependency treegraph
 
Reported: 2011-09-23 10:15 PDT by :Ehsan Akhgari
Modified: 2012-10-03 17:42 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP (17.33 KB, patch)
2011-09-23 10:19 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Patch (v1) (17.96 KB, patch)
2011-10-18 09:52 PDT, :Ehsan Akhgari
roc: review+
Details | Diff | Splinter Review

Description :Ehsan Akhgari 2011-09-23 10:15:48 PDT
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.
Comment 1 :Ehsan Akhgari 2011-09-23 10:19:15 PDT
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.
Comment 2 Mozilla RelEng Bot 2011-09-24 03:10:46 PDT
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 Mozilla RelEng Bot 2011-10-17 01:10:51 PDT
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
Comment 4 :Ehsan Akhgari 2011-10-18 09:52:42 PDT
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.
Comment 5 Timothy Nikkel (:tnikkel) 2011-10-18 12:24:31 PDT
How much of the win of lazy frame construction does resolving these style contexts take away?
Comment 6 :Ehsan Akhgari 2011-10-18 13:57:00 PDT
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?
Comment 7 Mozilla RelEng Bot 2011-10-19 01:30:26 PDT
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
Comment 8 Marco Bonardo [::mak] 2011-10-19 03:21:42 PDT
https://hg.mozilla.org/mozilla-central/rev/7ec3daabbf47
Comment 9 Timothy Nikkel (:tnikkel) 2011-10-19 14:14:36 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #6)
> Do you have better ideas on what we can do here?

Nope.

Note You need to log in before you can comment on or make changes to this bug.