Thanks for the issue.
Could you please provide us any sample test file or URL to triage the issue from test team end.

Thanks,

Hi, thanks for the quick reply. We managed to build a minimal working example, which resembles what our application is doing (which is large and uses GWT).
The attached example uses javascript to hide / show table entries via css. To do this, each table row has its own style sheet, to be able to hide / show each individual row. To modify the state of a row, the corresponding style is accessed from javascrip like this:
$doc.styleSheets[sheet].cssRules[ruleIndex].style[key] = val;  Unfortunately, as usual, this is old code and the person responsible for it left the company:-(. It is our understanding that at the time the code was written, this was the fastest method to hide / show a large number of entries, much faster than modifying the dom tree.

To reproduce the problem: Extract the attached .zip file, open test.html in Chrome (obvious I guess but don't forget the --allow-file-access-from-files parameter) and use the "hide" and "display" buttons at the top of the page.
In Chrome 56, the response to the click is instantly, in newer Chrome versions it takes almost 27 seconds.

Thanks a lot for your help!

Deleted: forgoogle.zip 44.4 KB

forgoogle.zip 44.4 KB Download

Thank you for providing more feedback. Adding requester "kavvaru@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Thanks for the test case. Adding needs-bisect label so we can try to find out where it started to get slow.

So I looked into this a bit, and it seems the slowness comes from rebuilding the active stylesheet list every time you index into document.styleSheets (since we invalidate it each time we change a property via DidMutateRules).

If I modify test.html as the attachment shows (grabbing the stylesheets off-hand instead of indexing each loop iteration), it becomes fast again.

I'm not too familiar regarding the active stylesheet list, but it seems that:

 * Calling document.styleSheets[i], calls into StylesheetList::item[1].
 * Which ends up in StyleEngine::StyleSheetsForStyleSheetList[2], where there's a TODO(rune@) that reads:
     we could split styleSheets and active stylesheet update to have a lighter update while accessing the styleSheets list.
 * That ends up calling UpdateActiveStyle(), which ends up rebuilding all the RuleSets if needed in DocumentStyleSheetCollection.

We then mutate the rules of the stylesheet, which makes us clear the RuleSet via WillMutateRules[3], and invalidate the active stylesheet list via DidMutateRules at[3], and clear the RuleSet at [4].

Then, we goto 1.

Rune, I'm not an expert in this code, did I get the diagnosis right? Any idea? Perhaps the easiest is just addressing your TODO. Happy to give it a try in case you're busy and have some pointers for me, btw.

Meanwhile, the attached change to the page should make perf acceptable again.

[1]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/StyleSheetList.cpp?l=47&rcl=52eb504ead78db774e2e21307e6322c1f7ce867f
[2]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/StyleEngine.cpp?l=121&rcl=73b8f7ed8d1105af3d39563f30a2771aee24923f
[3]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/CSSStyleSheet.cpp?l=149&rcl=73b8f7ed8d1105af3d39563f30a2771aee24923f
[4]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/CSSStyleSheet.cpp?l=176&rcl=73b8f7ed8d1105af3d39563f30a2771aee24923f

Deleted: test.html 67.2 KB

test.html 67.2 KB View Download

Able to reproduce this issue on windows 7, Mac 10.12.4,Linux Ubuntu 14.04 with Chrome stable version-58.0.3029.96 and Canary-60.0.3091.0 as per info provided in comment#3.
Manual Bisect:
-------------
Good-57.0.2953.0-Revision-438989
Bad-57.0.2954.0 -Revision-439279

Per Revision Bisect Tool Info:
-----------------------------
You are probably looking for a change made after 439091 (known good), but no later than 439092 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
https://chromium.googlesource.com/chromium/src/+log/206fee6c2a7ddb33c400a56adda7f53bf62157c2..90d4ea3d543f0031769b3aacac2d3e084b95fb7d

Possible suspect:
----------------
https://chromium.googlesource.com/chromium/src/+/90d4ea3d543f0031769b3aacac2d3e084b95fb7d

Review-Url: https://codereview.chromium.org/2557533005

Rune@, Kindly take a look and please help us to reassign this issue to a right owner if not with respect to this change.

Thanks.!

ecobos@: thanks for the analysis! I think we should address the TODO.

There's a chain of CLs to fix this ending with this one:

https://codereview.chromium.org/2884993002/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d4c419700c0edc7602c27d29a5b520f00bbe13a

commit 9d4c419700c0edc7602c27d29a5b520f00bbe13a
Author: rune <rune@opera.com>
Date: Tue May 16 17:10:02 2017

Avoid synchronous stylesheet update on html import loaded.

A use counter was updating the styleSheets list, which updated all of
active style to figure out if an html import contains stylesheets.
Instead, do a simpler walk of the stylesheet candidate nodes and return
early if one of them has a sheet, or a sheet load is in progress.

BUG= 717506 

Review-Url: https://codereview.chromium.org/2882983002
Cr-Commit-Position: refs/heads/master@{#472139}

[modify] https://crrev.com/9d4c419700c0edc7602c27d29a5b520f00bbe13a/third_party/WebKit/Source/core/dom/StyleEngine.h
[modify] https://crrev.com/9d4c419700c0edc7602c27d29a5b520f00bbe13a/third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollection.cpp
[modify] https://crrev.com/9d4c419700c0edc7602c27d29a5b520f00bbe13a/third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollection.h
[modify] https://crrev.com/9d4c419700c0edc7602c27d29a5b520f00bbe13a/third_party/WebKit/Source/core/html/imports/HTMLImportChild.cpp
[modify] https://crrev.com/9d4c419700c0edc7602c27d29a5b520f00bbe13a/third_party/WebKit/Source/web/BUILD.gn
[add] https://crrev.com/9d4c419700c0edc7602c27d29a5b520f00bbe13a/third_party/WebKit/Source/web/tests/HTMLImportSheetsTest.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/30a51b712800d2197a211b058dadb4a2c7762fed

commit 30a51b712800d2197a211b058dadb4a2c7762fed
Author: rune <rune@opera.com>
Date: Wed May 17 14:19:15 2017

Clear document scope dirtiness in import StyleEngine.

This is part of the plan to fix 717506 by re-collect sheets for
style_sheets_for_style_sheet_list_  separately from updating all of
active style to make it more light-weight.

Some sanity checking and comment about document scope dirtiness in,
and clear dirtiness after updating active sheets for, html imports.

Also renamed to UpdateActiveStyleSheetsInImport() to make it clearer
what the method does.

BUG= 717506 

Review-Url: https://codereview.chromium.org/2880263002
Cr-Commit-Position: refs/heads/master@{#472447}

[modify] https://crrev.com/30a51b712800d2197a211b058dadb4a2c7762fed/third_party/WebKit/Source/core/dom/DocumentStyleSheetCollection.cpp
[modify] https://crrev.com/30a51b712800d2197a211b058dadb4a2c7762fed/third_party/WebKit/Source/core/dom/StyleEngine.cpp
[modify] https://crrev.com/30a51b712800d2197a211b058dadb4a2c7762fed/third_party/WebKit/Source/core/dom/StyleEngine.h
[modify] https://crrev.com/30a51b712800d2197a211b058dadb4a2c7762fed/third_party/WebKit/Source/web/tests/HTMLImportSheetsTest.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5049772af9303fbb1dcdfe233900dfc80700ca1

commit b5049772af9303fbb1dcdfe233900dfc80700ca1
Author: rune <rune@opera.com>
Date: Thu May 18 09:23:35 2017

Update styleSheets list in import without active style update.

Querying document.styleSheets should not need to do a full active style
update. This CL is implementing a light-weight update of the styleSheets
list querying document.styleSheets on html import documents.

We collect and swap style_sheets_for_style_sheet_list_ for the
collection of the import document without touching the active style or
the dirty flags for active style on the master document. This is
straightforward for import documents as they don't have an active
stylesheet list themselves. Doing this optimization for top level
documents and shadow trees is the next step, but we need to be more
careful for those cases to keep the dirtyness without having to re-
collect for the styleSheets api every time.

BUG= 717506 

Review-Url: https://codereview.chromium.org/2880303002
Cr-Commit-Position: refs/heads/master@{#472751}

[modify] https://crrev.com/b5049772af9303fbb1dcdfe233900dfc80700ca1/third_party/WebKit/Source/core/dom/StyleEngine.cpp
[modify] https://crrev.com/b5049772af9303fbb1dcdfe233900dfc80700ca1/third_party/WebKit/Source/core/dom/StyleEngine.h
[modify] https://crrev.com/b5049772af9303fbb1dcdfe233900dfc80700ca1/third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollection.cpp
[modify] https://crrev.com/b5049772af9303fbb1dcdfe233900dfc80700ca1/third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollection.h
[modify] https://crrev.com/b5049772af9303fbb1dcdfe233900dfc80700ca1/third_party/WebKit/Source/web/tests/HTMLImportSheetsTest.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1719ffa41c44d3434095850fee7ffa5b6e57c4f

commit e1719ffa41c44d3434095850fee7ffa5b6e57c4f
Author: rune <rune@opera.com>
Date: Tue May 23 10:08:28 2017

Don't trigger full active style update on styleSheets access.

Element.styleSheets and ShadowRoot.styleSheets need to be made up-to-
date on access. We used to do a full active style update, but re-
collecting the stylesheet list should be enough, leaving the active
style dirty flags intact.

We introduce a dirty-flag for the stylesheet list in
StyleSheetCollection to avoid repeatedly re-collecting this list while
the active style is still dirty.

This coincidentally fixes  issue 722826  since we do not collect
stylesheets in import shadow trees as part of the active style update,
but is now made up-to-date on request when accessing the styleSheets
collection on shadow roots inside import documents.

This fixes the performance  issue 717506 .

R=meade@chromium.org
BUG= 717506 , 722826 

Review-Url: https://codereview.chromium.org/2884993002
Cr-Commit-Position: refs/heads/master@{#473846}

[add] https://crrev.com/e1719ffa41c44d3434095850fee7ffa5b6e57c4f/third_party/WebKit/LayoutTests/fast/html/imports/import-css-sheet-in-shadow.html
[modify] https://crrev.com/e1719ffa41c44d3434095850fee7ffa5b6e57c4f/third_party/WebKit/Source/core/dom/StyleEngine.cpp
[modify] https://crrev.com/e1719ffa41c44d3434095850fee7ffa5b6e57c4f/third_party/WebKit/Source/core/dom/StyleEngine.h
[modify] https://crrev.com/e1719ffa41c44d3434095850fee7ffa5b6e57c4f/third_party/WebKit/Source/core/dom/StyleEngineTest.cpp
[modify] https://crrev.com/e1719ffa41c44d3434095850fee7ffa5b6e57c4f/third_party/WebKit/Source/core/dom/StyleSheetCollection.cpp
[modify] https://crrev.com/e1719ffa41c44d3434095850fee7ffa5b6e57c4f/third_party/WebKit/Source/core/dom/StyleSheetCollection.h
[modify] https://crrev.com/e1719ffa41c44d3434095850fee7ffa5b6e57c4f/third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollection.cpp
[modify] https://crrev.com/e1719ffa41c44d3434095850fee7ffa5b6e57c4f/third_party/WebKit/Source/core/dom/TreeScopeStyleSheetCollection.h
[modify] https://crrev.com/e1719ffa41c44d3434095850fee7ffa5b6e57c4f/third_party/WebKit/Source/web/tests/HTMLImportSheetsTest.cpp

Verified the issue on windows 10, Mac 10.12.4 and Ubuntu 14.04 using chrome dev version #60.0.3112.7 as per comment #0 and #3.

Observed that response to the click on pressing "hide" and "display" button is happening instantly. Hence, the fix is working as expected.

Attaching screen cast for reference.

Hence, adding the verified labels.

Thanks...!!

Deleted: 717506.mp4 2.4 MB

	717506.mp4 2.4 MB View Download