Include disabled stylesheets in document.styleSheets.

Include disabled stylesheets in document.styleSheets.

Stylesheets disabled through LinkStyle.disabled were not included in
Document.styleSheets, even if they existed as CSSStyleSheet objects.

Fixed LinkStyle::setDisabled so it propagates disabled state to the sheet
object when present.

According to the HTML5 spec, setting disabled on LinkStyle when the sheet is
not present should not have any effect and getting it should return false. I
did not dare to do any changes there in this CL.

BUG=88310
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=153485

[dglazkov, 2013-06-26 15:56:27]

lgtm

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
File Source/core/dom/DocumentStyleSheetCollection.cpp (left):

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
Source/core/dom/DocumentStyleSheetCollection.cpp:416: if (sheets[i]->disabled())
How did this used to work with <link>?

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
File Source/core/dom/DocumentStyleSheetCollection.cpp (right):

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
Source/core/dom/DocumentStyleSheetCollection.cpp:296: if
(!linkElement->isDisabled() && linkElement->styleSheetIsLoading()) {
This needs to be untangled too, at some point. We should be just checking
sheet->disabled() for all these uniformly.

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
Source/core/dom/DocumentStyleSheetCollection.cpp:438: Vector<RefPtr<StyleSheet>
> cssStyleSheets;
Does "css" prefix carry any information here? Maybe we could get rid of it?

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
Source/core/dom/DocumentStyleSheetCollection.cpp:463:
InspectorInstrumentation::activeStyleSheetsUpdated(m_document, cssStyleSheets);
I guess the name of this function should change too, since we're feeding it all
stylesheets now.

https://codereview.chromium.org/17827004/diff/1/Source/core/html/HTMLLinkElem...
File Source/core/html/HTMLLinkElement.cpp (right):

https://codereview.chromium.org/17827004/diff/1/Source/core/html/HTMLLinkElem...
Source/core/html/HTMLLinkElement.cpp:535: m_sheet->setDisabled(disabled);
Jesus.

[rune, 2013-06-28 09:14:37]

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
File Source/core/dom/DocumentStyleSheetCollection.cpp (left):

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
Source/core/dom/DocumentStyleSheetCollection.cpp:416: if (sheets[i]->disabled())
On 2013/06/26 15:56:27, Dimitri Glazkov wrote:
> How did this used to work with <link>?

It would be caught by the code I removed from collectActiveStyleSheets by
checking the disabled state stored in LinkStyle instead:

if (linkElement->isDisabled())
  continue;

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
File Source/core/dom/DocumentStyleSheetCollection.cpp (right):

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
Source/core/dom/DocumentStyleSheetCollection.cpp:296: if
(!linkElement->isDisabled() && linkElement->styleSheetIsLoading()) {
On 2013/06/26 15:56:27, Dimitri Glazkov wrote:
> This needs to be untangled too, at some point. We should be just checking
> sheet->disabled() for all these uniformly.

Yes, according to the specs, disabled state should be stored on the sheet and
setting/getting should only have an effect when the sheet exists. I was afraid
of site-compat timing issues masked by the state on LinkStyle.

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
Source/core/dom/DocumentStyleSheetCollection.cpp:438: Vector<RefPtr<StyleSheet>
> cssStyleSheets;
On 2013/06/26 15:56:27, Dimitri Glazkov wrote:
> Does "css" prefix carry any information here? Maybe we could get rid of it?

Possibly not. According to the CSSOM spec, StyleSheetList contains CSS style
sheets. DOM 2 Style spec doesn't limit it to CSS. I've experimented a bit with
adding xslt stylesheets. Seems they're parsed as CSS(!) when added to HTML
document. That's out of scope for this CL, I think. Regardless, I'll remove
"css" from the name.

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
Source/core/dom/DocumentStyleSheetCollection.cpp:463:
InspectorInstrumentation::activeStyleSheetsUpdated(m_document, cssStyleSheets);
On 2013/06/26 15:56:27, Dimitri Glazkov wrote:
> I guess the name of this function should change too, since we're feeding it
all
> stylesheets now.

Yes. So this changes which stylesheets are passed to the inspector. Do you know
if that is a wanted change?

[rune, 2013-06-28 09:37:25]

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
File Source/core/dom/DocumentStyleSheetCollection.cpp (right):

https://codereview.chromium.org/17827004/diff/1/Source/core/dom/DocumentStyle...
Source/core/dom/DocumentStyleSheetCollection.cpp:463:
InspectorInstrumentation::activeStyleSheetsUpdated(m_document, cssStyleSheets);
On 2013/06/28 09:14:37, rune wrote:
> On 2013/06/26 15:56:27, Dimitri Glazkov wrote:
> > I guess the name of this function should change too, since we're feeding it
> all
> > stylesheets now.
> 
> Yes. So this changes which stylesheets are passed to the inspector. Do you
know
> if that is a wanted change?

Also, changing this method name means changing InspectorInstrumentation.idl. I
guess there is Inspector code relying on the method names in this API?

[dglazkov, 2013-06-28 14:58:16]

On 2013/06/28 09:37:25, rune wrote: 
> Also, changing this method name means changing InspectorInstrumentation.idl. I
> guess there is Inspector code relying on the method names in this API?

Let's not stress about this, then :)

[rune, 2013-07-02 12:21:10]

On 2013/06/28 14:58:16, Dimitri Glazkov wrote:
> On 2013/06/28 09:37:25, rune wrote: 
> > Also, changing this method name means changing InspectorInstrumentation.idl.
I
> > guess there is Inspector code relying on the method names in this API?
> 
> Let's not stress about this, then :)

@dglazkov: I've removed the "css" part of the variable name and rebased onto
master. Further comments?

[dglazkov, 2013-07-02 14:08:35]

On 2013/07/02 12:21:10, rune wrote:
> @dglazkov: I've removed the "css" part of the variable name and rebased onto
> master. Further comments?

lgtm++.

[commit-bot: I haz the power, 2013-07-02 19:06:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/17827004/14001

[commit-bot: I haz the power, 2013-07-02 20:51:48]

Retried try job too often on mac_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...

[commit-bot: I haz the power, 2013-07-02 21:23:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/17827004/14001

[commit-bot: I haz the power, 2013-07-03 07:23:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rune@opera.com/17827004/35001

[commit-bot: I haz the power, 2013-07-03 08:48:46]

Change committed as 153485