[UI-side compositing] Assertion in PlatformCAFilters::setFiltersOnLayer with animated...

https://bugs.webkit.org/show_bug.cgi?id=132528
<rdar://problem/16671660>

Reviewed by Tim Horton.

Source/WebKit2:
Allow PASSTHROUGH filters to be encoded and sent to the UI process; they can be set
on layers as the result of a filter animation using a reference filter, and just get
ignored anyway, but encoding them maintains consistency of the filters list.

* Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<FilterOperation>::encode): Assert during encoding if
we try to encode a NONE or REFERENCE filter (to match the decoding assertions).
(IPC::decodeFilterOperation): Allow decoding of PASSTHROUGH filters. Have
trying to decode a NONE or REFERENCE filter mark the message as invalid.
(IPC::ArgumentCoder<IDBKeyData>::decode): Mark the message invalid when receiving
unexpected key types.
* Shared/mac/RemoteLayerTreeTransaction.mm:
(WebKit::RemoteLayerTreeTextStream::operator<<): Have the logging not crash if
a filter is null (should never happen).

LayoutTests:
Make the animation duration a little longer to cause bug 132528 to reproduce more
reliably. The test does notifyDone() from an animation start event, so this doesn't
increase test duration.

* css3/filters/crash-filter-animation-invalid-url.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@168228 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index be1f1ca..9bf85b5 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,5 +1,19 @@
 2014-05-03  Simon Fraser  <simon.fraser@apple.com>
 
 2014-05-03  Simon Fraser  <simon.fraser@apple.com>
 

+        [UI-side compositing] Assertion in PlatformCAFilters::setFiltersOnLayer with animated reference filter
+        https://bugs.webkit.org/show_bug.cgi?id=132528
+        <rdar://problem/16671660>
+
+        Reviewed by Tim Horton.
+        
+        Make the animation duration a little longer to cause bug 132528 to reproduce more
+        reliably. The test does notifyDone() from an animation start event, so this doesn't
+        increase test duration.
+
+        * css3/filters/crash-filter-animation-invalid-url.html:
+
+2014-05-03  Simon Fraser  <simon.fraser@apple.com>
+

         Very fuzzy layers under non-decompasable matrices
         https://bugs.webkit.org/show_bug.cgi?id=132516
         <rdar://problem/16717478>
         Very fuzzy layers under non-decompasable matrices
         https://bugs.webkit.org/show_bug.cgi?id=132516
         <rdar://problem/16717478>

diff --git a/LayoutTests/css3/filters/crash-filter-animation-invalid-url.html b/LayoutTests/css3/filters/crash-filter-animation-invalid-url.html
index d867c23..f4fe0f4 100644 (file)
--- a/LayoutTests/css3/filters/crash-filter-animation-invalid-url.html
+++ b/LayoutTests/css3/filters/crash-filter-animation-invalid-url.html
@@ -3,7 +3,7 @@
 <style>
     a {
         -webkit-animation-name: n;
 <style>
     a {
         -webkit-animation-name: n;

-        -webkit-animation-duration: .01s;
+        -webkit-animation-duration: 50ms;

     }
     @-webkit-keyframes n {
         from {
     }
     @-webkit-keyframes n {
         from {

diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index ba234d2..43f3075 100644 (file)
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,26 @@
+2014-05-03  Simon Fraser  <simon.fraser@apple.com>
+
+        [UI-side compositing] Assertion in PlatformCAFilters::setFiltersOnLayer with animated reference filter
+        https://bugs.webkit.org/show_bug.cgi?id=132528
+        <rdar://problem/16671660>
+
+        Reviewed by Tim Horton.
+        
+        Allow PASSTHROUGH filters to be encoded and sent to the UI process; they can be set
+        on layers as the result of a filter animation using a reference filter, and just get
+        ignored anyway, but encoding them maintains consistency of the filters list.
+
+        * Shared/WebCoreArgumentCoders.cpp:
+        (IPC::ArgumentCoder<FilterOperation>::encode): Assert during encoding if
+        we try to encode a NONE or REFERENCE filter (to match the decoding assertions).
+        (IPC::decodeFilterOperation): Allow decoding of PASSTHROUGH filters. Have
+        trying to decode a NONE or REFERENCE filter mark the message as invalid.
+        (IPC::ArgumentCoder<IDBKeyData>::decode): Mark the message invalid when receiving
+        unexpected key types.
+        * Shared/mac/RemoteLayerTreeTransaction.mm:
+        (WebKit::RemoteLayerTreeTextStream::operator<<): Have the logging not crash if
+        a filter is null (should never happen).
+

 2014-05-02  Dan Bernstein  <mitz@apple.com>
 
         [Cocoa] Remove unused WKErrorRecoveryAttempting
 2014-05-02  Dan Bernstein  <mitz@apple.com>
 
         [Cocoa] Remove unused WKErrorRecoveryAttempting

diff --git a/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp b/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp
index 1269eaf..2e18427 100644 (file)
--- a/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp
+++ b/Source/WebKit2/Shared/WebCoreArgumentCoders.cpp
@@ -1561,10 +1561,10 @@ void ArgumentCoder<FilterOperation>::encode(ArgumentEncoder& encoder, const Filt
     encoder.encodeEnum(filter.type());
 
     switch (filter.type()) {
     encoder.encodeEnum(filter.type());
 
     switch (filter.type()) {

-    case FilterOperation::REFERENCE: {
+    case FilterOperation::NONE:
+    case FilterOperation::REFERENCE:

         ASSERT_NOT_REACHED();
         break;
         ASSERT_NOT_REACHED();
         break;

-    }

     case FilterOperation::GRAYSCALE:
     case FilterOperation::SEPIA:
     case FilterOperation::SATURATE:
     case FilterOperation::GRAYSCALE:
     case FilterOperation::SEPIA:
     case FilterOperation::SATURATE:


@@ -1591,7 +1591,6 @@ void ArgumentCoder<FilterOperation>::encode(ArgumentEncoder& encoder, const Filt
         encoder.encodeEnum(toDefaultFilterOperation(filter).representedType());
         break;
     case FilterOperation::PASSTHROUGH:
         encoder.encodeEnum(toDefaultFilterOperation(filter).representedType());
         break;
     case FilterOperation::PASSTHROUGH:

-    case FilterOperation::NONE:

         break;
     }
 }
         break;
     }
 }


@@ -1603,11 +1602,11 @@ bool decodeFilterOperation(ArgumentDecoder& decoder, RefPtr<FilterOperation>& fi
         return false;
 
     switch (type) {
         return false;
 
     switch (type) {

-    case FilterOperation::PASSTHROUGH:

     case FilterOperation::NONE:
     case FilterOperation::REFERENCE:
         ASSERT_NOT_REACHED();
     case FilterOperation::NONE:
     case FilterOperation::REFERENCE:
         ASSERT_NOT_REACHED();

-        break;
+        decoder.markInvalid();
+        return false;

     case FilterOperation::GRAYSCALE:
     case FilterOperation::SEPIA:
     case FilterOperation::SATURATE:
     case FilterOperation::GRAYSCALE:
     case FilterOperation::SEPIA:
     case FilterOperation::SATURATE:


@@ -1655,6 +1654,9 @@ bool decodeFilterOperation(ArgumentDecoder& decoder, RefPtr<FilterOperation>& fi
         filter = DefaultFilterOperation::create(representedType);
         break;
     }
         filter = DefaultFilterOperation::create(representedType);
         break;
     }

+    case FilterOperation::PASSTHROUGH:
+        filter = PassthroughFilterOperation::create();
+        break;

     }
             
     return true;
     }
             
     return true;


@@ -1835,6 +1837,7 @@ bool ArgumentCoder<IDBKeyData>::decode(ArgumentDecoder& decoder, IDBKeyData& key
         // MaxType and MinType are only used for comparison to other keys.
         // They should never be sent across the wire.
         ASSERT_NOT_REACHED();
         // MaxType and MinType are only used for comparison to other keys.
         // They should never be sent across the wire.
         ASSERT_NOT_REACHED();

+        decoder.markInvalid();

         return false;
     }
 
         return false;
     }
 

diff --git a/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm b/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm
index 8da6a60..d57ffe4 100644 (file)
--- a/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm
+++ b/Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm
@@ -646,7 +646,10 @@ RemoteLayerTreeTextStream& RemoteLayerTreeTextStream::operator<<(const FilterOpe
     RemoteLayerTreeTextStream& ts = *this;
     for (size_t i = 0; i < filters.size(); ++i) {
         const auto filter = filters.at(i);
     RemoteLayerTreeTextStream& ts = *this;
     for (size_t i = 0; i < filters.size(); ++i) {
         const auto filter = filters.at(i);

-        ts << *filter;
+        if (filter)
+            ts << *filter;
+        else
+            ts << "(null)";

         if (i < filters.size() - 1)
             ts << " ";
     }
         if (i < filters.size() - 1)
             ts << " ";
     }