⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 291749 in webkit


Ignore:
Timestamp:
Mar 23, 2022, 10:12:07 AM (5 years ago)
Author:
commit-queue@webkit.org
Message:

After losing context due to too many contexts, getError() does not return CONTEXT_LOST_WEBGL
​https://bugs.webkit.org/show_bug.cgi?id=236965

Patch by Kimmo Kinnunen <​kkinnunen@apple.com> on 2022-03-23
Reviewed by Kenneth Russell.

Source/WebCore:

After generating context lost, getError() is specified to return:

  • CONTEXT_LOST_WEBGL for first call
  • NO_ERROR for all the next calls.

WEBGL_lose_context is specified to add INVALID_OPERATION errors
even after context lost.

Change the code so that CONTEXT_LOST_WEBGL and WEBGL_lose_context induced
INVALID_OPERATION errors go to error vector in context lost -specific state.

Previously, these errors went into the m_context error vector. This is problematic
especially in the case where context loss happens where the m_context gets destroyed --
the error vector would be gone. This kind of loss happens for example when contexts
get lost due to the process having too many active contexts (least active context is "recycled").

Previously, any synthetized error was potentially obtainable after context lost. This is problematic
as it is not as specified. As mentioned above, only errors allowed after context lost is

  • CONTEXT_LOST_WEBGL first after context lost
  • WEBGL_lose_context.loseContext() and WEBGL_lose_context.restoreContext() induced INVALID_OPERATIONs

Changes the behavior to not report INVALID_OPERATION error in the theoretical case where we fail to
instantiate a new context. This is not allowed by the spec. Instead, just print an error to the console.

No new tests, updates the expectations of old ones with less failures.

  • html/canvas/WebGLRenderingContextBase.cpp:

(WebCore::WebGLRenderingContextBase::initializeNewContext):
(WebCore::WebGLRenderingContextBase::getError):
(WebCore::WebGLRenderingContextBase::isContextLost const):
(WebCore::WebGLRenderingContextBase::isContextLostOrPending):
(WebCore::WebGLRenderingContextBase::forceLostContext):
(WebCore::WebGLRenderingContextBase::loseContextImpl):
(WebCore::WebGLRenderingContextBase::forceRestoreContext):
(WebCore::WebGLRenderingContextBase::isContextUnrecoverablyLost const):
(WebCore::WebGLRenderingContextBase::scheduleTaskToDispatchContextLostEvent):
(WebCore::WebGLRenderingContextBase::maybeRestoreContext):
(WebCore::WebGLRenderingContextBase::synthesizeGLError):
(WebCore::WebGLRenderingContextBase::synthesizeLostContextGLError):

  • html/canvas/WebGLRenderingContextBase.h:

(WebCore::WebGLRenderingContextBase::ContextLostState::ContextLostState):

Source/WebKit:

Remove recording of synthetic webgl context lost error from the proxy.
This is now recorded in the WebGLRenderingContextBase.

  • WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.cpp:

(WebKit::RemoteGraphicsContextGLProxy::synthesizeGLError):
(WebKit::RemoteGraphicsContextGLProxy::getError):

  • WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.h:

LayoutTests:

  • fast/canvas/webgl/lose-context-on-status-failure-expected.txt:
  • webgl/lose-context-after-context-lost-expected.txt:
  • webgl/max-active-contexts-webglcontextlost-prevent-default-expected.txt:
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r291748 r291749  
     12022-03-23  Kimmo Kinnunen  <kkinnunen@apple.com>
     2
     3        After losing context due to too many contexts, getError() does not return CONTEXT_LOST_WEBGL
     4        https://bugs.webkit.org/show_bug.cgi?id=236965
     5
     6        Reviewed by Kenneth Russell.
     7
     8        * fast/canvas/webgl/lose-context-on-status-failure-expected.txt:
     9        * webgl/lose-context-after-context-lost-expected.txt:
     10        * webgl/max-active-contexts-webglcontextlost-prevent-default-expected.txt:
     11
    1122022-03-23  Kimmo Kinnunen  <kkinnunen@apple.com>
    213

  • trunk/LayoutTests/fast/canvas/webgl/lose-context-on-status-failure-expected.txt

    r259900 r291749  
    1 CONSOLE MESSAGE: WebGL: CONTEXT_LOST_WEBGL: loseContext: context lost
    2 CONSOLE MESSAGE: WebGL: CONTEXT_LOST_WEBGL: loseContext: context lost
    3 CONSOLE MESSAGE: WebGL: CONTEXT_LOST_WEBGL: loseContext: context lost
    4 CONSOLE MESSAGE: WebGL: CONTEXT_LOST_WEBGL: loseContext: context lost
     1CONSOLE MESSAGE: WebGL: context lost.
     2CONSOLE MESSAGE: WebGL: context lost.
     3CONSOLE MESSAGE: WebGL: context lost.
     4CONSOLE MESSAGE: WebGL: context lost.
    55Checks that a GPU status check failure will lose the context.
    66NOTE: This only passes in the test harness because it requires Internals.

  • trunk/LayoutTests/webgl/lose-context-after-context-lost-expected.txt

    r291399 r291749  
    33On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
    44
    5 TEST COMPLETE: 29 PASS, 2 FAIL
     5TEST COMPLETE: 31 PASS, 0 FAIL
    66
    77Running test: loseMethod: loseContext, testedMethod: loseContext
    … …  
    2020PASS Got webglcontextlost.
    2121PASS gl.isContextLost() is true
    22 FAIL gl.getError() should be 37442. Was 0.
     22PASS gl.getError() is gl.CONTEXT_LOST_WEBGL
    2323PASS gl.getError() is gl.NO_ERROR
    2424PASS Did not crash on tested method loseContext.
    … …  
    2626PASS Got webglcontextlost.
    2727PASS gl.isContextLost() is true
    28 FAIL gl.getError() should be 37442. Was 0.
     28PASS gl.getError() is gl.CONTEXT_LOST_WEBGL
    2929PASS gl.getError() is gl.NO_ERROR
    3030PASS Did not crash on tested method restoreContext.

  • trunk/LayoutTests/webgl/max-active-contexts-webglcontextlost-prevent-default-expected.txt

    r291477 r291749  
    33On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
    44
    5 TEST COMPLETE: 42 PASS, 10 FAIL
     5TEST COMPLETE: 46 PASS, 6 FAIL
    66
    77Running test: loseMethod: loseContext, loseMethod2: loseContext
    … …  
    1515PASS getError was expected value: CONTEXT_LOST_WEBGL :
    1616PASS gl.isContextLost() is true
    17 PASS getError was expected value: NO_ERROR :
     17FAIL getError expected: NO_ERROR. Was INVALID_OPERATION :
    1818Running test: loseMethod: loseContext, loseMethod2: gpuStatusFailure
    1919PASS Got webglcontextlost and restore was attempted.
    … …  
    2828Running test: loseMethod: manyContexts, loseMethod2: loseContext
    2929PASS Got webglcontextlost and restore was attempted.
    30 FAIL getError expected: CONTEXT_LOST_WEBGL. Was NO_ERROR :
    31 FAIL getError expected: INVALID_OPERATION. Was NO_ERROR :
     30PASS getError was expected value: CONTEXT_LOST_WEBGL :
     31PASS getError was expected value: INVALID_OPERATION :
    3232PASS gl.isContextLost() is true
    3333PASS getError was expected value: NO_ERROR :
    3434Running test: loseMethod: manyContexts, loseMethod2: manyContexts
    3535PASS Got webglcontextlost and restore was attempted.
    36 FAIL getError expected: CONTEXT_LOST_WEBGL. Was NO_ERROR :
     36PASS getError was expected value: CONTEXT_LOST_WEBGL :
    3737PASS gl.isContextLost() is true
    3838PASS getError was expected value: NO_ERROR :
    3939Running test: loseMethod: manyContexts, loseMethod2: gpuStatusFailure
    4040PASS Got webglcontextlost and restore was attempted.
    41 FAIL getError expected: CONTEXT_LOST_WEBGL. Was NO_ERROR :
     41PASS getError was expected value: CONTEXT_LOST_WEBGL :
    4242PASS gl.isContextLost() is true
    4343PASS getError was expected value: NO_ERROR :
    4444Running test: loseMethod: manyContexts, loseMethod2: nothing
    4545PASS Got webglcontextlost and restore was attempted.
    46 FAIL getError expected: CONTEXT_LOST_WEBGL. Was NO_ERROR :
     46PASS getError was expected value: CONTEXT_LOST_WEBGL :
    4747PASS gl.isContextLost() is true
    4848PASS getError was expected value: NO_ERROR :

  • trunk/Source/WebCore/ChangeLog

    r291748 r291749  
     12022-03-23  Kimmo Kinnunen  <kkinnunen@apple.com>
     2
     3        After losing context due to too many contexts, getError() does not return CONTEXT_LOST_WEBGL
     4        https://bugs.webkit.org/show_bug.cgi?id=236965
     5
     6        Reviewed by Kenneth Russell.
     7
     8        After generating context lost, getError() is specified to return:
     9         - CONTEXT_LOST_WEBGL for first call
     10         - NO_ERROR for all the next calls.
     11
     12        WEBGL_lose_context is specified to add INVALID_OPERATION errors
     13        even after context lost.
     14
     15        Change the code so that CONTEXT_LOST_WEBGL and WEBGL_lose_context induced
     16        INVALID_OPERATION errors go to error vector in context lost -specific state.
     17
     18        Previously, these errors went into the m_context error vector. This is problematic
     19        especially in the case where context loss happens where the m_context gets destroyed --
     20        the error vector would be gone. This kind of loss happens for example when contexts
     21        get lost due to the process having too many active contexts (least active context is "recycled").
     22
     23        Previously, any synthetized error was potentially obtainable after context lost. This is problematic
     24        as it is not as specified. As mentioned above, only errors allowed after context lost is
     25         - CONTEXT_LOST_WEBGL first after context lost
     26         - WEBGL_lose_context.loseContext() and WEBGL_lose_context.restoreContext() induced INVALID_OPERATIONs
     27
     28        Changes the behavior to not report INVALID_OPERATION error in the theoretical case where we fail to
     29        instantiate a new context. This is not allowed by the spec. Instead, just print an error to the console.
     30
     31        No new tests, updates the expectations of old ones with less failures.
     32
     33        * html/canvas/WebGLRenderingContextBase.cpp:
     34        (WebCore::WebGLRenderingContextBase::initializeNewContext):
     35        (WebCore::WebGLRenderingContextBase::getError):
     36        (WebCore::WebGLRenderingContextBase::isContextLost const):
     37        (WebCore::WebGLRenderingContextBase::isContextLostOrPending):
     38        (WebCore::WebGLRenderingContextBase::forceLostContext):
     39        (WebCore::WebGLRenderingContextBase::loseContextImpl):
     40        (WebCore::WebGLRenderingContextBase::forceRestoreContext):
     41        (WebCore::WebGLRenderingContextBase::isContextUnrecoverablyLost const):
     42        (WebCore::WebGLRenderingContextBase::scheduleTaskToDispatchContextLostEvent):
     43        (WebCore::WebGLRenderingContextBase::maybeRestoreContext):
     44        (WebCore::WebGLRenderingContextBase::synthesizeGLError):
     45        (WebCore::WebGLRenderingContextBase::synthesizeLostContextGLError):
     46        * html/canvas/WebGLRenderingContextBase.h:
     47        (WebCore::WebGLRenderingContextBase::ContextLostState::ContextLostState):
     48
    1492022-03-23  Kimmo Kinnunen  <kkinnunen@apple.com>
    250

  • trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp

    r291611 r291749  
    10471047void WebGLRenderingContextBase::initializeNewContext()
    10481048{
    1049     ASSERT(!m_contextLost);
     1049    ASSERT(!isContextLost());
    10501050    m_needsUpdate = true;
    10511051    m_markedCanvasDirty = false;
    … …  
    31673167GCGLenum WebGLRenderingContextBase::getError()
    31683168{
    3169     if (!m_context || m_isPendingPolicyResolution)
     3169    if (isContextLost()) {
     3170        auto& errors = m_contextLostState->errors;
     3171        if (!errors.isEmpty())
     3172            return errors.takeFirst();
     3173        return GraphicsContextGL::NO_ERROR;
     3174    }
     3175    if (m_isPendingPolicyResolution)
    31703176        return GraphicsContextGL::NO_ERROR;
    31713177    return m_context->getError();
    … …  
    40564062bool WebGLRenderingContextBase::isContextLost() const
    40574063{
    4058     return m_contextLost;
     4064    return m_contextLostState.has_value();
    40594065}
    40604066
    … …  
    40774083    }
    40784084
    4079     return m_contextLost || m_isPendingPolicyResolution;
     4085    return isContextLost() || m_isPendingPolicyResolution;
    40804086}
    40814087
    … …  
    64346440{
    64356441    if (isContextLostOrPending()) {
    6436         synthesizeGLError(GraphicsContextGL::INVALID_OPERATION, "loseContext", "context already lost");
     6442        synthesizeLostContextGLError(GraphicsContextGL::INVALID_OPERATION, "loseContext", "context already lost");
    64376443        return;
    64386444    }
    … …  
    64456451    if (isContextLost())
    64466452        return;
    6447 
    6448     m_contextLost = true;
    6449     m_contextLostMode = mode;
     6453    if (mode == RealLostContext)
     6454        printToConsole(MessageLevel::Error, "WebGL: context lost.");
     6455
     6456    m_contextLostState = ContextLostState { mode };
     6457    m_contextLostState->errors.add(GraphicsContextGL::CONTEXT_LOST_WEBGL);
    64506458
    64516459    detachAndRemoveAllObjects();
    … …  
    64606468            break;
    64616469    }
    6462     ConsoleDisplayPreference display = (mode == RealLostContext) ? DisplayInConsole: DontDisplayInConsole;
    6463     synthesizeGLError(GraphicsContextGL::CONTEXT_LOST_WEBGL, "loseContext", "context lost", display);
    6464 
    6465     // Don't allow restoration unless the context lost event has both been
    6466     // dispatched and its default behavior prevented.
    6467     m_restoreAllowed = false;
    64686470
    64696471    // Always defer the dispatch of the context lost event, to implement
    … …  
    64786480        return;
    64796481    }
    6480 
    6481     if (!m_restoreAllowed) {
    6482         if (m_contextLostMode == SyntheticLostContext)
    6483             synthesizeGLError(GraphicsContextGL::INVALID_OPERATION, "restoreContext", "context restoration not allowed");
     6482    if (!m_contextLostState->restoreRequested) {
     6483        if (m_contextLostState->mode == SyntheticLostContext)
     6484            synthesizeLostContextGLError(GraphicsContextGL::INVALID_OPERATION, "restoreContext", "context restoration not allowed");
    64846485        return;
    64856486    }
    … …  
    64916492bool WebGLRenderingContextBase::isContextUnrecoverablyLost() const
    64926493{
    6493     return m_contextLost && !m_restoreAllowed;
     6494    return isContextLost() && !m_contextLostState->restoreRequested;
    64946495}
    64956496
    … …  
    77337734        if (isContextStopped())
    77347735            return;
    7735 
     7736        if (!isContextLost())
     7737            return;
    77367738        auto event = WebGLContextEvent::create(eventNames().webglcontextlostEvent, Event::CanBubble::No, Event::IsCancelable::Yes, emptyString());
    77377739        canvas->dispatchEvent(event);
    7738         m_restoreAllowed = event->defaultPrevented();
    7739         if (m_contextLostMode == RealLostContext && m_restoreAllowed)
     7740        m_contextLostState->restoreRequested = event->defaultPrevented();
     7741        if (m_contextLostState->mode == RealLostContext && m_contextLostState->restoreRequested)
    77407742            m_restoreTimer.startOneShot(0_s);
    77417743    });
    … …  
    77457747{
    77467748    RELEASE_ASSERT(!m_isSuspended);
    7747     ASSERT(m_contextLost);
    7748     if (!m_contextLost)
    7749         return;
    7750 
    7751     // The rendering context is not restored unless the default behavior of the
    7752     // webglcontextlost event was prevented earlier.
    7753     //
    7754     // Because of the way m_restoreTimer is set up for real vs. synthetic lost
    7755     // context events, we don't have to worry about this test short-circuiting
    7756     // the retry loop for real context lost events.
    7757     if (!m_restoreAllowed)
    7758         return;
     7749    if (!isContextLost() || !m_contextLostState->restoreRequested) {
     7750        ASSERT_NOT_REACHED();
     7751        return;
     7752    }
    77597753
    77607754    auto* canvas = htmlCanvas();
    … …  
    77817775    RefPtr<GraphicsContextGL> context = hostWindow->createGraphicsContextGL(m_attributes);
    77827776    if (!context) {
    7783         if (m_contextLostMode == RealLostContext)
     7777        if (m_contextLostState->mode == RealLostContext)
    77847778            m_restoreTimer.startOneShot(secondsBetweenRestoreAttempts);
    77857779        else
    7786             // This likely shouldn't happen but is the best way to report it to the WebGL app.
    7787             synthesizeGLError(GraphicsContextGL::INVALID_OPERATION, "", "error restoring context");
     7780            printToConsole(MessageLevel::Error, "WebGL: error restoring lost context.");
    77887781        return;
    77897782    }
    … …  
    77917784    setGraphicsContextGL(context.releaseNonNull());
    77927785    addActivityStateChangeObserverIfNecessary();
    7793     m_contextLost = false;
     7786    m_contextLostState = std::nullopt;
    77947787    setupFlags();
    77957788    initializeNewContext();
    … …  
    78737866} // namespace anonymous
    78747867
    7875 void WebGLRenderingContextBase::synthesizeGLError(GCGLenum error, const char* functionName, const char* description, ConsoleDisplayPreference display)
    7876 {
    7877     if (m_synthesizedErrorsToConsole && display == DisplayInConsole) {
    7878         String str = "WebGL: " + GetErrorString(error) +  ": " + String(functionName) + ": " + String(description);
    7879         printToConsole(MessageLevel::Error, str);
    7880     }
     7868void WebGLRenderingContextBase::synthesizeGLError(GCGLenum error, const char* functionName, const char* description)
     7869{
     7870    printToConsole(MessageLevel::Error, makeString("WebGL: ", GetErrorString(error), ": ", functionName, ": ", description));
    78817871    if (m_context)
    78827872        m_context->synthesizeGLError(error);
     7873}
     7874
     7875void WebGLRenderingContextBase::synthesizeLostContextGLError(GCGLenum error, const char* functionName, const char* description)
     7876{
     7877    printToConsole(MessageLevel::Error, makeString("WebGL: ", GetErrorString(error), ": ", functionName, ": ", description));
     7878    m_contextLostState->errors.add(error);
    78837879}
    78847880

  • trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h

    r291468 r291749  
    4949#include <memory>
    5050#include <wtf/CheckedArithmetic.h>
     51#include <wtf/ListHashSet.h>
    5152#include <wtf/Lock.h>
    5253
    … …  
    549550    void updateActiveOrdinal();
    550551
     552    struct ContextLostState {
     553        ContextLostState(LostContextMode mode)
     554            : mode(mode)
     555        {
     556        }
     557        ListHashSet<GCGLint> errors; // Losing context and WEBGL_lose_context generates errors here.
     558        LostContextMode mode { LostContextMode::RealLostContext };
     559        bool restoreRequested { false };
     560    };
     561
    551562    RefPtr<GraphicsContextGL> m_context;
    552563    RefPtr<WebGLContextGroup> m_contextGroup;
    553564    Lock m_objectGraphLock;
    554565
    555     bool m_restoreAllowed { false };
    556566    SuspendableTimer m_restoreTimer;
    557567
    … …  
    653663    GCGLenum m_unpackColorspaceConversion;
    654664
    655     bool m_contextLost { false };
    656     LostContextMode m_contextLostMode { SyntheticLostContext };
     665    std::optional<ContextLostState>  m_contextLostState;
    657666    WebGLContextAttributes m_attributes;
    658667
    … …  
    10961105
    10971106    // Wrapper for GraphicsContextGLOpenGL::synthesizeGLError that sends a message to the JavaScript console.
    1098     enum ConsoleDisplayPreference { DisplayInConsole, DontDisplayInConsole };
    1099     void synthesizeGLError(GCGLenum, const char* functionName, const char* description, ConsoleDisplayPreference = DisplayInConsole);
     1107    void synthesizeGLError(GCGLenum, const char* functionName, const char* description);
     1108    void synthesizeLostContextGLError(GCGLenum, const char* functionName, const char* description);
    11001109
    11011110    String ensureNotNull(const String&) const;

  • trunk/Source/WebKit/ChangeLog

    r291739 r291749  
     12022-03-23  Kimmo Kinnunen  <kkinnunen@apple.com>
     2
     3        After losing context due to too many contexts, getError() does not return CONTEXT_LOST_WEBGL
     4        https://bugs.webkit.org/show_bug.cgi?id=236965
     5
     6        Reviewed by Kenneth Russell.
     7
     8        Remove recording of synthetic webgl context lost error from the proxy.
     9        This is now recorded in the WebGLRenderingContextBase.
     10
     11        * WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.cpp:
     12        (WebKit::RemoteGraphicsContextGLProxy::synthesizeGLError):
     13        (WebKit::RemoteGraphicsContextGLProxy::getError):
     14        * WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.h:
     15
    1162022-03-23  Fujii Hironori  <Hironori.Fujii@sony.com>
    217

  • trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.cpp

    r291468 r291749  
    216216        return;
    217217    }
    218     m_errorWhenContextIsLost = error;
    219218}
    220219
    … …  
    228227        return static_cast<GCGLenum>(returnValue);
    229228    }
    230     return std::exchange(m_errorWhenContextIsLost, NO_ERROR);
     229    return NO_ERROR;
    231230}
    232231

  • trunk/Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.h

    r291477 r291749  
    367367
    368368    HashSet<String> m_enabledExtensions;
    369     GCGLenum m_errorWhenContextIsLost = NO_ERROR;
    370369    IPC::StreamClientConnection m_streamConnection;
    371370};
Note: See TracChangeset for help on using the changeset viewer.