[webkit-reviews] review denied: [Bug 202065] REGRESSION(r243264): Web Inspector: Style pane doesn't update after toggling CSS class : [Attachment 379420] Patch for bots

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has denied Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 202065: REGRESSION(r243264): Web Inspector: Style pane doesn't update after
toggling CSS class
https://bugs.webkit.org/show_bug.cgi?id=202065

Attachment 379420: Patch for bots

https://bugs.webkit.org/attachment.cgi?id=379420&action=review




--- Comment #10 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 379420
  --> https://bugs.webkit.org/attachment.cgi?id=379420
Patch for bots

View in context: https://bugs.webkit.org/attachment.cgi?id=379420&action=review

r-, we don't want to just revert these changes, as IIRC they were necessary in
order for r243264 to work correctly.

(In reply to Nikita Vasilyev from comment #2)
> r243264 removed code that cleared the maps. After r243264, maps don't get
cleared.
> 
>	   function fetchedMatchedStyles(error, matchedRulesPayload,
pseudoElementRulesPayload, inheritedRulesPayload)
>	   {
> @@ -142,20 +188,12 @@ WI.DOMNodeStyles = class DOMNodeStyles extends
WI.Object
>	       pseudoElementRulesPayload = pseudoElementRulesPayload || [];
>	       inheritedRulesPayload = inheritedRulesPayload || [];
>  
> -	       // Move the current maps to previous.
> -	       this._previousRulesMap = this._rulesMap;
> -	       this._previousStyleDeclarationsMap = this._styleDeclarationsMap;
> -
> -	       // Clear the current maps.
> -	       this._rulesMap = {};
> -	       this._styleDeclarationsMap = {};
> 
> Because of this, WI.DOMNodeStyles.Event.Refreshed gets called with
significantChange set to false. significantChange can be true only when a style
rule is new (and not present in previousStylesMap).
> 
> The style rules only get updated when WI.DOMNodeStyles.Event.Refreshed is
fired with significantChange set to true.
If that's the case, can we change the logic for how `significantChange` is
determined instead?  It seems like that is the real issue here, not how (or
what) we decide to keep in a map.

> Source/WebInspectorUI/ChangeLog:9
> +	   Revert all changes related to `_styleDeclarationsMap`.

Why does this solve the problem?  Please add some description either to your
investigation or why/how you arrived at this conclusion in the ChangeLog.