[rule view] Don't use the hidden window for creating CSS value nodes

RESOLVED FIXED in Firefox 42

Status

DevTools
Inspector
RESOLVED FIXED
3 years ago
a month ago

People

(Reporter: Simon Lindholm, Assigned: Simon Lindholm)

Tracking

unspecified
Firefox 42
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
Because it's unportable, and the resulting cross-document node adoptions are quite slow. This reduces rule view update times by ~5-10% in my tests.
(Assignee)

Comment 1

3 years ago
Created attachment 8638908 [details] [diff] [review]
0002-Bug-1187584-Don-t-use-the-hidden-window-for-creating.patch

green try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0e4caf27df7

I could remove the " || Services.appShell.hiddenDOMWindow.document;" if wanted; the only reasons not to is that a test lacked a reasonable document to pass in, and the mentality that I still keep from Firebug never to change a public API. (Though mxr-addons doesn't show any hits, so eh.)
Attachment #8638908 - Flags: review?(mratcliffe)
Comment on attachment 8638908 [details] [diff] [review]
0002-Bug-1187584-Don-t-use-the-hidden-window-for-creating.patch

Review of attachment 8638908 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Simon Lindholm from comment #1)
> Created attachment 8638908 [details] [diff] [review]
> 0002-Bug-1187584-Don-t-use-the-hidden-window-for-creating.patch
> 
> green try:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0e4caf27df7
> 
> I could remove the " || Services.appShell.hiddenDOMWindow.document;" if
> wanted; the only reasons not to is that a test lacked a reasonable document
> to pass in, and the mentality that I still keep from Firebug never to change
> a public API. (Though mxr-addons doesn't show any hits, so eh.)

Yes, please remove it... you will need to run it through try again but everybody knows I hate these hiddenDOMWindow 'cos it is always a hack.
Attachment #8638908 - Flags: review?(mratcliffe)
(Assignee)

Comment 3

3 years ago
Created attachment 8638989 [details] [diff] [review]
0001-Bug-1187584-Don-t-use-the-hidden-window-for-creating.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=751807ba9876

(on top of some other commits on inbound, somehow. let's hope "document" is always valid in mochitest-chrome tests.)
Attachment #8638908 - Attachment is obsolete: true
Attachment #8638989 - Flags: review?(mratcliffe)
Attachment #8638989 - Flags: review?(mratcliffe) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
sorry this failed to apply:

Hunk #1 FAILED at 134
1 out of 1 hunks FAILED -- saving rejects to file browser/devtools/styleinspector/computed-view.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh 0001-Bug-1187584-Don-t-use-the-hidden-window-for-creating.patch 

could you take a look, thanks!
Flags: needinfo?(simon.lindholm10)
Keywords: checkin-needed
(Assignee)

Comment 5

3 years ago
Created attachment 8639932 [details] [diff] [review]
0001-Bug-1187584-Don-t-use-the-hidden-window-for-creating.patch

Trivial rebase
Attachment #8638989 - Attachment is obsolete: true
Flags: needinfo?(simon.lindholm10)
Attachment #8639932 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2c18a7c06a48
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42

Updated

a month ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.