Bug 193149 - Web Inspector: Audit: create new IDL type for exposing special functionality in test context
Summary: Web Inspector: Audit: create new IDL type for exposing special functionality ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: WebInspectorAuditTab
Blocks: 193225 193226 193227 193228 193262
  Show dependency treegraph
 
Reported: 2019-01-04 11:56 PST by Devin Rousso
Modified: 2019-01-15 10:03 PST (History)
9 users (show)

See Also:


Attachments
Patch (91.19 KB, patch)
2019-01-04 12:22 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (2.77 MB, application/zip)
2019-01-04 13:25 PST, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.13 MB, application/zip)
2019-01-04 13:42 PST, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-sierra (2.26 MB, application/zip)
2019-01-04 14:48 PST, Build Bot
no flags Details
Patch (112.11 KB, patch)
2019-01-04 15:19 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-sierra (2.81 MB, application/zip)
2019-01-04 16:36 PST, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-sierra-wk2 (3.96 MB, application/zip)
2019-01-04 16:59 PST, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-sierra (2.17 MB, application/zip)
2019-01-04 18:06 PST, Build Bot
no flags Details
Patch (112.75 KB, patch)
2019-01-04 19:15 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (134.36 KB, patch)
2019-01-06 11:14 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (134.21 KB, patch)
2019-01-06 13:09 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (134.48 KB, patch)
2019-01-10 15:27 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (134.72 KB, patch)
2019-01-13 11:21 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (134.36 KB, patch)
2019-01-14 19:07 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (134.33 KB, patch)
2019-01-14 19:27 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-highsierra (2.47 MB, application/zip)
2019-01-14 20:37 PST, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-highsierra (2.01 MB, application/zip)
2019-01-14 21:24 PST, Build Bot
no flags Details
Patch (134.34 KB, patch)
2019-01-14 23:08 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-01-04 11:56:14 PST
In order to provide special functionality within Audit tests, we need a way to expose references to these functions without having them be visible when not running an Audit.
Comment 1 Devin Rousso 2019-01-04 11:56:27 PST
<rdar://problem/46801218>
Comment 2 Devin Rousso 2019-01-04 12:22:44 PST
Created attachment 358347 [details]
Patch
Comment 3 Devin Rousso 2019-01-04 12:31:09 PST
(In reply to Devin Rousso from comment #2)
> Created attachment 358347 [details]
> Patch
One thing I was thinking about was instead of only providing `AUDIT` when running within a Page, we could always provide `AUDIT` as an empty object and instead make each of the sub-objects have a separate IDL (e.g. `InspectorAuditDOMUtitlities`, `InspectorAuditAccessibilityUtilities`, etc), adding them to the `AUDIT` object as necessary (e.g. Page vs Worker vs JSContext).  I don't feel strongly either way, other than having more IDL this way.

Another idea I'd pondered with was to skip the whole IDL idea altogether and just manually create the `AUDIT` object and sub-objects, providing the functions as `JSNativeStdFunction`.  The downside of this approach is that we lose the "free" bits of the IDL generator (namely the type checking and conversions).
Comment 4 Joseph Pecoraro 2019-01-04 13:21:00 PST
Comment on attachment 358347 [details]
Patch

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

> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:76
> +    String functionString = makeString("(function(", argumentsString, ") { \"use strict\"; return eval(", test, ")(); })");

Why commas around `test` in the eval?

> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:98
> +    return { };

Should JSContexts even have an AuditAgent if we aren't going to expose an AUDIT object to it?

> Source/JavaScriptCore/inspector/protocol/Audit.json:4
> +    "domain": "Audit",
> +    "description": "",
> +    "commands": [

Does this make sense to expose to a "javascript", "worker", or "service-worker" context? I think it makes sense to restrict Audits to a "page" right now.

> Source/WebCore/inspector/InspectorController.cpp:152
> +    m_agents.append(std::make_unique<PageAuditAgent>(pageContext));

This should go under `createLazyAgents` since there is no need for it to be alive before a frontend connects.

> Source/WebCore/inspector/WorkerInspectorController.cpp:81
> +    m_agents.append(std::make_unique<WorkerAuditAgent>(workerContext));

Ditto: `createLazyAgents`

> Source/WebCore/inspector/agents/page/PageAuditAgent.cpp:86
> +                    inspectorAuditUtilities = toJSNewlyCreated(execState, globalObject, InspectorAuditUtilities::create());

We could avoid creating an AUDIT object each time. For example, InjectedScriptHost is created once per target. (See calls to InjectedScriptHost::create).

> Source/WebInspectorUI/UserInterface/Protocol/Target.js:55
> +        console.assert(this.AuditAgent);

We wouldn't have an AuditAgent in every target (an older iOS target for example).

> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:62
> +        WI.auditManager.addEventListener(WI.AuditManager.Event.TestScheduled, this._handleAuditManagerTestScheduled, this);
> +        WI.auditManager.addEventListener(WI.AuditManager.Event.TestCompleted, this._handleAuditManagerTestCompleted, this);

This sounds worth doing separate from this patch, right?
Comment 5 Build Bot 2019-01-04 13:24:59 PST Comment hidden (obsolete)
Comment 6 Build Bot 2019-01-04 13:25:01 PST Comment hidden (obsolete)
Comment 7 Build Bot 2019-01-04 13:42:53 PST Comment hidden (obsolete)
Comment 8 Build Bot 2019-01-04 13:42:55 PST Comment hidden (obsolete)
Comment 9 Devin Rousso 2019-01-04 14:11:58 PST
Comment on attachment 358347 [details]
Patch

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

>> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:76
>> +    String functionString = makeString("(function(", argumentsString, ") { \"use strict\"; return eval(", test, ")(); })");
> 
> Why commas around `test` in the eval?

This is adding the `test` variable to the `functionString`, not the literal string "test".  One thing to note, however, is that there should be quotes around `test`.

    String functionString = makeString("(function(", argumentsString, ") { \"use strict\"; return eval(\"", test, "\")(); })");

>> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:98
>> +    return { };
> 
> Should JSContexts even have an AuditAgent if we aren't going to expose an AUDIT object to it?

In the future there might be other things we'd want to make available via an Audit (e.g. promise resolution state) that could be useful when writing tests.

I don't have a good reason to disallow Audits in non-Page contexts, especially considering the fact that there's nothing really preventing that from happening.
Comment 10 Devin Rousso 2019-01-04 14:14:55 PST
Comment on attachment 358347 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:62
>> +        WI.auditManager.addEventListener(WI.AuditManager.Event.TestCompleted, this._handleAuditManagerTestCompleted, this);
> 
> This sounds worth doing separate from this patch, right?

<https://webkit.org/b/193158> Web Inspector: Audit: disable breakpoints when running Audit
Comment 11 Build Bot 2019-01-04 14:48:07 PST Comment hidden (obsolete)
Comment 12 Build Bot 2019-01-04 14:48:09 PST Comment hidden (obsolete)
Comment 13 Devin Rousso 2019-01-04 15:19:01 PST
Created attachment 358380 [details]
Patch
Comment 14 Build Bot 2019-01-04 16:36:40 PST Comment hidden (obsolete)
Comment 15 Build Bot 2019-01-04 16:36:42 PST Comment hidden (obsolete)
Comment 16 Build Bot 2019-01-04 16:59:36 PST Comment hidden (obsolete)
Comment 17 Build Bot 2019-01-04 16:59:38 PST Comment hidden (obsolete)
Comment 18 Matt Baker 2019-01-04 17:12:32 PST
Comment on attachment 358380 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:39
> +        Create a version of `evaluate` that accepts a list of arguemnts to be passed in as arguments

arguemnts -> arguments
Comment 19 Build Bot 2019-01-04 18:06:44 PST Comment hidden (obsolete)
Comment 20 Build Bot 2019-01-04 18:06:46 PST Comment hidden (obsolete)
Comment 21 Devin Rousso 2019-01-04 19:15:15 PST
Created attachment 358422 [details]
Patch
Comment 22 Devin Rousso 2019-01-06 11:14:23 PST
Created attachment 358462 [details]
Patch

Rework the injected `AUDIT` object to be a plain JavaScript object with special IDL subobjects.  This way, we can always provide an `AUDIT` object, and can call the `AuditAgent` functions in such an order as to preserve the `AUDIT` object between tests, allowing for a simple way to share values between runs.
Comment 23 Devin Rousso 2019-01-06 13:09:26 PST
Created attachment 358467 [details]
Patch
Comment 24 Devin Rousso 2019-01-10 15:27:33 PST
Created attachment 358843 [details]
Patch

Add a few extra functions for use by later patches
Comment 25 Devin Rousso 2019-01-11 14:35:14 PST
Comment on attachment 358843 [details]
Patch

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

> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:102
> +    functionString.appendLiteral(")`)(); })");

We should also add another "AUDIT" inside the `()` so that if the test function string defines `AUDIT` as the first parameter, it isn't overwritten.
Comment 26 Devin Rousso 2019-01-11 14:36:35 PST
Comment on attachment 358843 [details]
Patch

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

> Source/WebCore/inspector/agents/page/PageAuditAgent.cpp:85
> +                    m_audit##name##Utilities = InspectorAudit##name##Utilities::create(); \

Since both `InspectorAuditAccessibilityUtilites` and `InspectorAuditDOMUtilities` will passed `*this` (in order to check whether a test is currently running, and to throw an error if not), we should make that stub implementation now so the future patches don't have to worry about it.
Comment 27 Devin Rousso 2019-01-13 11:21:55 PST
Created attachment 359007 [details]
Patch
Comment 28 Joseph Pecoraro 2019-01-14 13:51:26 PST
Comment on attachment 359007 [details]
Patch

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

r- just because I'd like to see an updated version given all of the things I've pointed out below.

• The tests look good.
• I'm still not convinced that `AUDIT.Domain` is the best approach for an API but lets proceed with it for now. I'd have preferred something like something like "WebInspectorAudit" instead of "AUDIT".

> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:1248
> +		91278D5821DEDA9600B57184 /* JSGlobalObjectAuditAgent.h in Headers */ = {isa = PBXBuildFile; fileRef = 91278D5621DEDA9400B57184 /* JSGlobalObjectAuditAgent.h */; settings = {ATTRIBUTES = (Private, ); }; };

This fix should not be Private, it isn't and won't be needed outside of JavaScriptCore.

> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:87
> +    for (auto& argument : runArguments(executionContextId))
> +        m_injectedAUDITValue->putDirect(vm, Identifier::fromString(execState, argument.first), argument.second);

I find the name `runArguments` to be very confusing. It sounds like an action but it isn't. How about something like `populateAuditObject`. And then you could just pass in the `m_injectedAUDITValue` instead of this weird and wasteful intermediate Vector.

> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:105
> +    StringBuilder functionString;
> +    functionString.appendLiteral("(function(");
> +    if (m_injectedAUDITValue)
> +        functionString.appendLiteral("AUDIT");
> +    functionString.appendLiteral(") { \"use strict\"; return eval(`(");
> +    functionString.append(test.isolatedCopy().replace('`', "\\`"));
> +    functionString.appendLiteral(")`)(");
> +    if (m_injectedAUDITValue)
> +        functionString.appendLiteral("AUDIT");
> +    functionString.appendLiteral("); })");

It might be nice to include a comment before this to show what it is doing in a less confusing way:

    // Perform the following:
    //
    //     (function(AUDIT) {
    //         "use strict";
    //         return eval(`(<escaped-test>)`(AUDIT);
    //     })

Is there any reason to not include "AUDIT" always? It would just be `undefined` if not provided below. I'd drop the `m_injectedAUDITValue` branches up here.

> Source/JavaScriptCore/inspector/agents/JSGlobalObjectAuditAgent.h:36
> +class JSGlobalObjectAuditAgent final : public InspectorAuditAgent {

I still think we should remove Audits from being available in JSContexts and Workers. Maybe it would work with a ESLint style audit, but in general I think it is not particularly useful there and would just confuse JSContext inspector users. That would eliminate quite a bit of this new code in JavaScriptCore. Note we should update the frontend to also not allow the Audits tab for JSContexts.

> Source/JavaScriptCore/inspector/protocol/Audit.json:22
> +            "name": "run",
> +            "description": "Parses and evaluates the given test string and sends back the result.",
> +            "parameters": [
> +                { "name": "test", "type": "string", "description": "Test string to parse and evaluate." },
> +                { "name": "contextId", "$ref": "Runtime.ExecutionContextId", "optional": true, "description": "Specifies in which isolated context to run the test. Each content script lives in an isolated context and this parameter may be used to specify one of those contexts. If the parameter is omitted or 0 the evaluation will be performed in the context of the inspected page." }
> +            ],
> +            "returns": [
> +                { "name": "result", "$ref": "Runtime.RemoteObject", "description": "Evaluation result." },
> +                { "name": "wasThrown", "type": "boolean", "optional": true, "description": "True if the result was thrown during the evaluation." }
> +            ]

It may be important to point out that this puts results into the "audit" object group. Which the frontend would need to release in order to free up memory from audits (or navigate the page) and thereby clear the entire injected script.

> Source/JavaScriptCore/inspector/protocol/Audit.json:26
> +            "description": ""

This could use a description. It seems like this may be necessary to balance `setup` sometimes.

> Source/WebCore/inspector/InspectorAuditAccessibilityUtilities.h:42
> +    explicit InspectorAuditAccessibilityUtilities();

Can you get away with `= default;` and drop the cpp file?

> Source/WebCore/inspector/WorkerInspectorController.cpp:181
> +    m_agents.append(std::make_unique<WorkerAuditAgent>(workerContext));

Ditto RE not including an Audit Agent in Workers unless there is a clear use case.

> Source/WebCore/inspector/agents/page/PageAuditAgent.cpp:66
> +        if (executionContextId)
> +            errorString = "Internal error: main world execution context not found."_s;

This should be `if (!executionContextId)` if the error is about the main world execution context.

> Source/WebCore/inspector/agents/page/PageAuditAgent.cpp:87
> +#define ADD_AUDIT_UTILITIES(name) \
> +                if (!m_audit##name##Utilities) \
> +                    m_audit##name##Utilities = InspectorAudit##name##Utilities::create(*this); \
> +                if (JSC::JSValue inspectorAudit##name##Utilities = toJS(execState, globalObject, *m_audit##name##Utilities)) \
> +                    arguments.append(std::make_pair("" #name ""_s, WTFMove(inspectorAudit##name##Utilities)));

I'd prefer an "Object" suffix to a "Utilities" suffix. Meaning the name "InspectorAudit<Foo>Object" instead of "InspectorAudit<Foo>Utilities".

We already have this pattern already in JavaScriptCore for the C++ backend to some JavaScript "Object":

    ./JavaScriptCore/runtime/SymbolObject.h
    ./JavaScriptCore/runtime/IntlObject.h
    ./JavaScriptCore/runtime/InspectorInstrumentationObject.h
    ./JavaScriptCore/runtime/ReflectObject.h
    ./JavaScriptCore/runtime/BigIntObject.h
    ./JavaScriptCore/runtime/ConsoleObject.h
    ./JavaScriptCore/runtime/StringObject.h
    ./JavaScriptCore/runtime/JSONObject.h
    ./JavaScriptCore/runtime/MathObject.h
    ...

In contrast to "Utilities" files which are often collections of functions that may be used to work with something:

    ./WebCore/page/Base64Utilities.h
    ./WebCore/platform/win/GDIUtilities.h
    ./WebCore/platform/network/mac/UTIUtilities.h
    ./WebCore/platform/animation/AnimationUtilities.h
    ./WebCore/platform/ios/wak/WKUtilities.h
    ./WebCore/platform/mac/StringUtilities.h
    ...

This falls more cleanly into the "Object" suffix bucket.

> Source/WebCore/inspector/agents/page/PageAuditAgent.cpp:111
> +void PageAuditAgent::muteConsole()
> +{
> +    InspectorAuditAgent::muteConsole();
> +
> +    PageConsoleClient::mute();
> +}
> +
> +void PageAuditAgent::unmuteConsole()
> +{
> +    PageConsoleClient::unmute();
> +
> +    InspectorAuditAgent::unmuteConsole();
> +}

Style: I'd remove the inner newlines and make these as compact as possible.

> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:239
> +            agentCommandArguments.objectGroup = "audit";

Do we ever clear the `audit` object group anywhere? Does the frontend clear/trash results anywhere?
Comment 29 Devin Rousso 2019-01-14 18:35:21 PST
Comment on attachment 359007 [details]
Patch

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

> • I'm still not convinced that `AUDIT.Domain` is the best approach for an API but lets proceed with it for now. I'd have preferred something like something like "WebInspectorAudit" instead of "AUDIT".
I like `WebInspectorAudit` over `AUDIT`.  It seems bit more "normal", in that we rarely use all-caps except for constants.  It's also a bit more "specific" than something general like "audit".

>> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:105
>> +    functionString.appendLiteral("); })");
> 
> It might be nice to include a comment before this to show what it is doing in a less confusing way:
> 
>     // Perform the following:
>     //
>     //     (function(AUDIT) {
>     //         "use strict";
>     //         return eval(`(<escaped-test>)`(AUDIT);
>     //     })
> 
> Is there any reason to not include "AUDIT" always? It would just be `undefined` if not provided below. I'd drop the `m_injectedAUDITValue` branches up here.

Good point.  There's no real reason to not always supply it.

>> Source/JavaScriptCore/inspector/protocol/Audit.json:26
>> +            "description": ""
> 
> This could use a description. It seems like this may be necessary to balance `setup` sometimes.

Ah whoops 😅

>> Source/WebCore/inspector/InspectorAuditAccessibilityUtilities.h:42
>> +    explicit InspectorAuditAccessibilityUtilities();
> 
> Can you get away with `= default;` and drop the cpp file?

The cpp file is used by all of the blocked patches, so rather than have them worry about it, I felt that it made sense to include it in the base patch (this way I wouldn't have to worry about what patches landed in what order).

>> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:239
>> +            agentCommandArguments.objectGroup = "audit";
> 
> Do we ever clear the `audit` object group anywhere? Does the frontend clear/trash results anywhere?

Ah, good point.  I think it's safe to clear them before each time we call `Audit.run`.
Comment 30 Devin Rousso 2019-01-14 19:07:47 PST
Created attachment 359118 [details]
Patch
Comment 31 Devin Rousso 2019-01-14 19:10:09 PST
Comment on attachment 359007 [details]
Patch

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

>>> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:239
>>> +            agentCommandArguments.objectGroup = "audit";
>> 
>> Do we ever clear the `audit` object group anywhere? Does the frontend clear/trash results anywhere?
> 
> Ah, good point.  I think it's safe to clear them before each time we call `Audit.run`.

Actually, since we have the "Results" folder, we shouldn't ever clear the object group, as we could potentially show a value at any point in the future.  Clearing this will happen on page navigation (unless we add a button somewhere).
Comment 32 Joseph Pecoraro 2019-01-14 19:23:09 PST
Comment on attachment 359118 [details]
Patch

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

r=me! Very nice

> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:102
> +    options.includeCommandLineAPI = true;

Should we be including the command line API?! That means they would have `getEventListeners` and we wouldn't need our (future) DOM additions. I feel like we shouldn't include the command line API.
Comment 33 Devin Rousso 2019-01-14 19:24:42 PST
Comment on attachment 359118 [details]
Patch

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

>> Source/JavaScriptCore/inspector/agents/InspectorAuditAgent.cpp:102
>> +    options.includeCommandLineAPI = true;
> 
> Should we be including the command line API?! That means they would have `getEventListeners` and we wouldn't need our (future) DOM additions. I feel like we shouldn't include the command line API.

Oh!  I was trying to allow for `console.*` functions.  My mistake.
Comment 34 Devin Rousso 2019-01-14 19:27:14 PST
Created attachment 359120 [details]
Patch
Comment 35 Build Bot 2019-01-14 20:37:05 PST Comment hidden (obsolete)
Comment 36 Build Bot 2019-01-14 20:37:07 PST Comment hidden (obsolete)
Comment 37 Build Bot 2019-01-14 21:24:57 PST Comment hidden (obsolete)
Comment 38 Build Bot 2019-01-14 21:24:59 PST Comment hidden (obsolete)
Comment 39 Devin Rousso 2019-01-14 23:08:22 PST
Created attachment 359139 [details]
Patch
Comment 40 WebKit Commit Bot 2019-01-15 00:25:40 PST
Comment on attachment 359139 [details]
Patch

Clearing flags on attachment: 359139

Committed r239976: <https://trac.webkit.org/changeset/239976>
Comment 41 WebKit Commit Bot 2019-01-15 00:25:42 PST
All reviewed patches have been landed.  Closing bug.
Comment 42 Truitt Savell 2019-01-15 09:10:52 PST
It appears that the changes in https://trac.webkit.org/changeset/239976/webkit

has caused inspector/model/remote-object.html to become a constant failure. 

History:
http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector%2Fmodel%2Fremote-object.html

Diff:
--- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/inspector/model/remote-object-expected.txt
+++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/inspector/model/remote-object-actual.txt
@@ -1729,7 +1729,7 @@
       {
         "_name": "stack",
         "_type": "string",
-        "_value": "splitText@[native code]\nglobal code\nevaluateWithSc…ension@[native code]\n\n_wrapCall"
+        "_value": "splitText@[native code]\nglobal code\nevaluateWithScopeExtension@[native code]\n\n_wrapCall"
       },
       {
         "_name": "code",


I confirmed this issue by reverting this revision locally and running the test. After the revert the test started passing again.
Comment 43 Truitt Savell 2019-01-15 09:12:23 PST
This looks like it could just be a rebaseline, could you confirm?
Comment 44 Devin Rousso 2019-01-15 10:03:32 PST
(In reply to Truitt Savell from comment #43)
> This looks like it could just be a rebaseline, could you confirm?
<https://trac.webkit.org/changeset/239990/webkit>