Bug 197561 – Implement "shared" compositing layers, allowing overlap layers to paint into the backing store of another layer

• Status: RESOLVED
• Resolution: FIXED
• Component: Compositing

[Simon Fraser (smfr)]

An "inclusive" compositing layer is one that can paint a set of other (usually sibling) layers that would otherwise be composited for overlap. They will reduce the amount of backing store, and the need for scrolling tree nodes in some cases.

[Radar WebKit Bug Importer]

<rdar://problem/50445998>

[Simon Fraser (smfr)]

Created attachment 369080 [details]
Patch

[Simon Fraser (smfr)]

*** Bug 197594 has been marked as a duplicate of this bug. ***

[EWS Watchlist]

Comment on attachment 369080 [details]
Patch

Attachment 369080 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12102569

New failing tests:
media/media-continues-playing-after-replace-source.html
media/media-ended.html

[EWS Watchlist]

Created attachment 369082 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369080 [details]
Patch

Attachment 369080 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12102575

New failing tests:
media/media-continues-playing-after-replace-source.html
media/media-ended.html

[EWS Watchlist]

Created attachment 369083 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369080 [details]
Patch

Attachment 369080 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12102571

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 369085 [details]
Archive of layout-test-results from ews114 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369080 [details]
Patch

Attachment 369080 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12102657

New failing tests:
security/contentSecurityPolicy/video-with-file-url-allowed-by-media-src-star.html

[EWS Watchlist]

Created attachment 369087 [details]
Archive of layout-test-results from ews214 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews214  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[EWS Watchlist]

Comment on attachment 369080 [details]
Patch

Attachment 369080 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12102636

New failing tests:
fast/scrolling/ios/overflow-scroll-overlap-4.html
media/media-continues-playing-after-replace-source.html
media/track/track-cues-enter-exit.html
media/media-ended.html
media/track/track-cues-cuechange.html

[EWS Watchlist]

Created attachment 369088 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.4

[Simon Fraser (smfr)]

Comment on attachment 369080 [details]
Patch

Crashes are because traverseUnchangedSubtree() needs to do sharing stuff.

[Antti Koivisto]

Comment on attachment 369080 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:1817
> +    RenderLayer* repaintTarget;
> +    if (includeSelf == IncludeSelf && (repaintTarget = repaintTargetForLayer(*this)))
> +        return repaintTarget;

No reason to leave repaintTarget uninitialized.

> Source/WebCore/rendering/RenderLayer.h:818
> +    // If non,null, a non-ancestor composited layer that this layer paints into (it is sharing its backing store with this layer).

"non-null"

> Source/WebCore/rendering/RenderLayer.h:822
> +    RenderLayer* sharedLayer() const { return m_sharedLayer; }
> +    void setSharedLayer(RenderLayer* sharedLayer) { m_sharedLayer = sharedLayer; }
> +
> +    bool paintsIntoSharedLayer() const { return m_sharedLayer; }

"Shared layer" does not really describe the feature. I feel it would be clearer to express this in terms of "shared backing" in code. This might be just a matter of renaming and probably also switching to RenderLayerBacking ptrs here.

I wonder if there some sort of future refactoring here that actually makes RenderLayerBacking a shared object?

> Source/WebCore/rendering/RenderLayer.h:1268
> +    // If non,null, a non-ancestor composited layer that this layer paints into.

"non-null"

> Source/WebCore/rendering/RenderLayerBacking.cpp:264
> +    for (auto* layer : m_sharingLayers)
> +        layer->setSharedLayer(nullptr);

This kind of stuff is scary. Layer tree should use WeakPtrs.

It would be better to call clearSharingLayers() to avoid duplication.

> Source/WebCore/rendering/RenderLayerBacking.cpp:2678
> +        for (auto* layer : m_sharingLayers)
> +            paintOneLayer(*layer, sharingLayerPaintFlags);

I suppose region gathering code needs to replicate this logic too. Please file a bug.

> Source/WebCore/rendering/RenderLayerBacking.h:398
> +    // When this layer is a shared layer, sharing layers other than m_owningLayer are in this list, and paint into our backing store.

This is a confusing sentence.

[Simon Fraser (smfr)]

(In reply to Antti Koivisto from comment #15)
> Comment on attachment 369080 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=369080&action=review
> 
> > Source/WebCore/rendering/RenderLayer.cpp:1817
> > +    RenderLayer* repaintTarget;
> > +    if (includeSelf == IncludeSelf && (repaintTarget = repaintTargetForLayer(*this)))
> > +        return repaintTarget;
> 
> No reason to leave repaintTarget uninitialized.
> 
> > Source/WebCore/rendering/RenderLayer.h:818
> > +    // If non,null, a non-ancestor composited layer that this layer paints into (it is sharing its backing store with this layer).
> 
> "non-null"
> 
> > Source/WebCore/rendering/RenderLayer.h:822
> > +    RenderLayer* sharedLayer() const { return m_sharedLayer; }
> > +    void setSharedLayer(RenderLayer* sharedLayer) { m_sharedLayer = sharedLayer; }
> > +
> > +    bool paintsIntoSharedLayer() const { return m_sharedLayer; }
> 
> "Shared layer" does not really describe the feature. I feel it would be
> clearer to express this in terms of "shared backing" in code. This might be
> just a matter of renaming and probably also switching to RenderLayerBacking
> ptrs here.

I agree that "shared" isn't too descriptive. Other implementations have called them "inclusive" which has clearer directionality ("inclusive, included") but Zalan didn't like that.

I don't want to use RLB pointers because I don't want to introduce a dependency on when backing actually gets created.

> I wonder if there some sort of future refactoring here that actually makes
> RenderLayerBacking a shared object?

Perhaps, though there is still a clear "primary" owner.

> > Source/WebCore/rendering/RenderLayer.h:1268
> > +    // If non,null, a non-ancestor composited layer that this layer paints into.
> 
> "non-null"
> 
> > Source/WebCore/rendering/RenderLayerBacking.cpp:264
> > +    for (auto* layer : m_sharingLayers)
> > +        layer->setSharedLayer(nullptr);
> 
> This kind of stuff is scary. Layer tree should use WeakPtrs.

Will fix.

> It would be better to call clearSharingLayers() to avoid duplication.
> 
> > Source/WebCore/rendering/RenderLayerBacking.cpp:2678
> > +        for (auto* layer : m_sharingLayers)
> > +            paintOneLayer(*layer, sharingLayerPaintFlags);
> 
> I suppose region gathering code needs to replicate this logic too. Please
> file a bug.

Good point.

> > Source/WebCore/rendering/RenderLayerBacking.h:398
> > +    // When this layer is a shared layer, sharing layers other than m_owningLayer are in this list, and paint into our backing store.
> 
> This is a confusing sentence.

Will fix.

[Antti Koivisto]

Comment on attachment 369080 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:4082
> +        paintFlags.add(PaintLayerTemporaryClipRects);

This disables clip caching right? Should we worry about performance?

[Simon Fraser (smfr)]

Created attachment 369134 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 369231 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 369232 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 369234 [details]
Patch

[EWS Watchlist]

Comment on attachment 369234 [details]
Patch

Attachment 369234 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12120290

New failing tests:
compositing/shared-layers/partial-compositing-update.html
media/modern-media-controls/volume-support/volume-support-drag-to-mute.html
media/modern-media-controls/volume-support/volume-support-drag.html
media/modern-media-controls/scrubber-support/scrubber-support-click.html
media/modern-media-controls/fullscreen-support/fullscreen-support-press.html
compositing/shared-layers/partial-compositing-update2.html
media/modern-media-controls/playback-support/playback-support-button-click.html
media/audio-concurrent-supported.html
media/modern-media-controls/placard-support/placard-support-pip.html
media/modern-media-controls/mute-support/mute-support-press-on-button.html
media/modern-media-controls/scrubber-support/scrubber-support-drag.html
media/modern-media-controls/volume-support/volume-support-click.html

[EWS Watchlist]

Created attachment 369243 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369234 [details]
Patch

Attachment 369234 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12120307

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 369245 [details]
Archive of layout-test-results from ews117 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369234 [details]
Patch

Attachment 369234 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12120358

New failing tests:
compositing/shared-layers/partial-compositing-update.html
media/modern-media-controls/volume-support/volume-support-drag-to-mute.html
media/modern-media-controls/volume-support/volume-support-drag.html
media/audio-concurrent-supported.html
media/modern-media-controls/fullscreen-support/fullscreen-support-press.html
media/audio-mpeg-supported.html
compositing/shared-layers/partial-compositing-update2.html
media/modern-media-controls/playback-support/playback-support-button-click.html
media/modern-media-controls/scrubber-support/scrubber-support-click.html
media/media-ended.html
media/media-continues-playing-after-replace-source.html
media/modern-media-controls/mute-support/mute-support-press-on-button.html
media/modern-media-controls/scrubber-support/scrubber-support-drag.html
media/modern-media-controls/volume-support/volume-support-click.html
media/media-ended-fired-once.html

[EWS Watchlist]

Created attachment 369246 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369234 [details]
Patch

Attachment 369234 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12120398

New failing tests:
compositing/shared-layers/partial-compositing-update.html
media/audio-mpeg4-supported.html
fast/scrolling/ios/overflow-scroll-overlap-4.html
media/audio-concurrent-supported.html
media/audio-playback-restriction-removed-muted.html
compositing/shared-layers/partial-compositing-update2.html
compositing/shared-layers/overflow-scroll/absolute-in-stacking-relative-in-scroller.html
media/media-continues-playing-after-replace-source.html
media/media-ended-fired-once.html

[EWS Watchlist]

Created attachment 369251 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.4

[EWS Watchlist]

Comment on attachment 369234 [details]
Patch

Attachment 369234 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12123996

New failing tests:
svg/repaint/remove-border-property-on-root.html

[EWS Watchlist]

Created attachment 369294 [details]
Archive of layout-test-results from ews210 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Simon Fraser (smfr)]

Created attachment 369301 [details]
Patch

[EWS Watchlist]

Comment on attachment 369301 [details]
Patch

Attachment 369301 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12125821

New failing tests:
fast/scrolling/ios/overflow-scroll-overlap-4.html
compositing/shared-layers/partial-compositing-update.html
compositing/shared-layers/overflow-scroll/absolute-in-stacking-relative-in-scroller.html
compositing/shared-layers/partial-compositing-update2.html

[EWS Watchlist]

Created attachment 369316 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.4

[EWS Watchlist]

Comment on attachment 369301 [details]
Patch

Attachment 369301 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12125836

New failing tests:
compositing/shared-layers/partial-compositing-update2.html
compositing/shared-layers/partial-compositing-update.html
media/modern-media-controls/volume-support/volume-support-drag-to-mute.html
media/modern-media-controls/volume-support/volume-support-drag.html
media/modern-media-controls/scrubber-support/scrubber-support-click.html
media/track/track-cue-rendering-snap-to-lines-not-set.html
fast/hidpi/video-controls-in-hidpi.html
media/modern-media-controls/playback-support/playback-support-button-click.html
media/video-zoom-controls.html
media/modern-media-controls/scrubber-support/scrubber-support-media-api.html
media/track/track-cue-rendering-vertical.html
media/modern-media-controls/mute-support/mute-support-press-on-button.html
media/modern-media-controls/scrubber-support/scrubber-support-drag.html
media/modern-media-controls/volume-support/volume-support-click.html
media/video-trackmenu-selection.html

[EWS Watchlist]

Created attachment 369320 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Simon Fraser (smfr)]

Created attachment 369355 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 369357 [details]
Patch

[EWS Watchlist]

Comment on attachment 369357 [details]
Patch

Attachment 369357 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12130330

New failing tests:
fast/hidpi/video-controls-in-hidpi.html

[EWS Watchlist]

Created attachment 369359 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369357 [details]
Patch

Attachment 369357 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12130361

New failing tests:
fast/scrolling/ios/overflow-scroll-overlap-4.html

[EWS Watchlist]

Created attachment 369360 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.4

[Antti Koivisto]

> New failing tests:
> fast/scrolling/ios/overflow-scroll-overlap-4.html

This is failing because of the missing region code.

[Antti Koivisto]

Comment on attachment 369357 [details]
Patch

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

Looks fine. Fixing updateEventRegion shouldn't be difficult either (same thing as paintIntoLayer) but it can be done separately if you prefer.

> Source/WebCore/rendering/RenderLayer.cpp:1774
> +//    ASSERT_IMPLIES(backingProvider, !isComposited());

Commented-out code.

> Source/WebCore/rendering/RenderLayerBacking.cpp:679
> +        if (!layerWeakPtr) {
> +            ASSERT_NOT_REACHED();
> +            continue;
> +        }

This seems overly defensive. We should just crash with nullptr if the logic is supposed to guarantee that these are non-null (here and elsewhere).

[EWS Watchlist]

Comment on attachment 369357 [details]
Patch

Attachment 369357 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12131207

New failing tests:
http/tests/css/filters-on-iframes.html

[EWS Watchlist]

Created attachment 369364 [details]
Archive of layout-test-results from ews210 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews210  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Simon Fraser (smfr)]

https://trac.webkit.org/r245058

[Alexey Proskuryakov]

> webkit.org/b/197695 [ Debug ] fast/hidpi/video-controls-in-hidpi.html [ Skip ]

This patch skipped an existing test. How is this OK?

[Ryan Haddad]

Reverted r245058 for reason:

Causes crashes under ASan / GuardMalloc

Committed r245085: <https://trac.webkit.org/changeset/245085>

[Simon Fraser (smfr)]

Re-landed with fixes in https://trac.webkit.org/changeset/245170/webkit

[Antti Koivisto]

This hits debug asserts on many sites (say, nytimes.com)

void RenderLayer::paintLayerContents(GraphicsContext& context, const LayerPaintingInfo& paintingInfo, OptionSet<PaintLayerFlag> paintFlags)
{
    ASSERT(isSelfPaintingLayer() || hasSelfPaintingLayerDescendant());


(lldb) bt 10
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
  * frame #0: 0x000000035a69285e JavaScriptCore`::WTFCrash() at Assertions.cpp:305:35
    frame #1: 0x000000035ee1ce2b WebCore`WTFCrashWithInfo((null)=4368, (null)="rendering/RenderLayer.cpp", (null)="void WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext &, const WebCore::RenderLayer::LayerPaintingInfo &, OptionSet<WebCore::RenderLayer::PaintLayerFlag>)", (null)=2340) at Assertions.h:568:5
    frame #2: 0x00000003622b75f6 WebCore`WebCore::RenderLayer::paintLayerContents(this=0x00000003946d8c18, context=0x00007ffee42dc358, paintingInfo=0x00007ffee42dbde0, paintFlags=(m_storage = 96)) at RenderLayer.cpp:4368:5
    frame #3: 0x00000003622d81ac WebCore`WebCore::RenderLayerBacking::paintIntoLayer(this=0x00007ffee42dbec8, layer=0x00000003946d8c18, paintFlags=(m_storage = 96))::$_9::operator()(WebCore::RenderLayer&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>) const at RenderLayerBacking.cpp:2674:15
    frame #4: 0x00000003622d7f70 WebCore`WebCore::RenderLayerBacking::paintIntoLayer(this=0x000000039460fa20, graphicsLayer=0x000000039473d000, context=0x00007ffee42dc358, paintDirtyRect=0x00007ffee42dc0d8, paintBehavior=(m_storage = 2048), paintingPhase='\x03') at RenderLayerBacking.cpp:2701:13
    frame #5: 0x00000003622d869d WebCore`WebCore::RenderLayerBacking::paintContents(this=0x000000039460fa20, graphicsLayer=0x000000039473d000, context=0x00007ffee42dc358, paintingPhase='\x03', clip=0x00007ffee42dc160, layerPaintBehavior=2) at RenderLayerBacking.cpp:2744:9
    frame #6: 0x0000000361de23b7 WebCore`WebCore::GraphicsLayer::paintGraphicsLayerContents(this=0x000000039473d000, context=0x00007ffee42dc358, clip=0x00000003947f49b8, layerPaintBehavior=2) at GraphicsLayer.cpp:501:14
    frame #7: 0x0000000361e5a8ad WebCore`WebCore::GraphicsLayerCA::platformCALayerPaintContents(this=0x000000039473d000, (null)=0x0000000392633000, context=0x00007ffee42dc358, clip=0x00000003947f49b8, layerPaintBehavior=2) at GraphicsLayerCA.cpp:1678:5
    frame #8: 0x000000035f4df4c3 WebCore`WebCore::PlatformCALayer::drawLayerContents(context=0x00007fc3dced2ba0, platformCALayer=0x0000000392633000, dirtyRects=0x00000003947f49a8, layerPaintBehavior=2) at PlatformCALayerCocoa.mm:1198:28
    frame #9: 0x000000010c5056f0 WebKit`WebKit::RemoteLayerBackingStore::drawInContext(this=0x00000003947f4958, context=0x000000039ff97840, backImage=0x0000000000000000) at RemoteLayerBackingStore.mm:355:9

[Simon Fraser (smfr)]

Filed bug 197776 on that.

[Michael Catanzaro]

[885/2246] Building CXX object Source/WebCore/CMakeFiles/...es/WebCore/unified-sources/UnifiedSource-043dd90b-8.cpp.o
In file included from DerivedSources/WebCore/unified-sources/UnifiedSource-043dd90b-8.cpp:5:
../../Source/WebCore/rendering/RenderLayer.cpp: In member function ‘WTF::Ref<WebCore::ClipRects> WebCore::RenderLayer::updateClipRects(const WebCore::RenderLayer::ClipRectsContext&)’:
../../Source/WebCore/rendering/RenderLayer.cpp:5503:15: warning: unused variable ‘parentLayer’ [-Wunused-variable]
 5503 |     if (auto* parentLayer = (clipRectsContext.rootLayer != this ? parent() : nullptr))
      |               ^~~~~~~~~~~

This condition here:

    RefPtr<ClipRects> parentClipRects;
    // For transformed layers, the root layer was shifted to be us, so there is no need to
    // examine the parent. We want to cache clip rects with us as the root.
    if (auto* parentLayer = (clipRectsContext.rootLayer != this ? parent() : nullptr))
        parentClipRects = this->parentClipRects(clipRectsContext);

Should now be simplified to:

    RefPtr<ClipRects> parentClipRects;
    // For transformed layers, the root layer was shifted to be us, so there is no need to
    // examine the parent. We want to cache clip rects with us as the root.
    if (clipRectsContext.rootLayer != this)
        parentClipRects = this->parentClipRects(clipRectsContext);

[Michael Catanzaro]

Er no, that's a disaster think-o, sorry. Should be simplified to:

if (clipRectsContext.rootLayer != this && parent())

[Michael Catanzaro]

Will fix in: bug #197785. (Assuming the new logic is correct and only needs to be simplified.)