Bug 200458 – Web Inspector: Implement `queryHolders` Command Line API

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

Call queryRetainers(object) from the Console to return an array of objects that strongly reference the given `object`.

This would be extremely useful for finding specific leaks (understanding how a particular object is kept alive).

[Devin Rousso]

Created attachment 375598 [details]
[Patch] WIP

This works, but it needs tests and a way of removing inspector objects (e.g. `InjectedScript`) from the list of retainers.

[EWS Watchlist]

Attachment 375598 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:124:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 157 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Devin Rousso]

Created attachment 375607 [details]
[Patch] WIP

[EWS Watchlist]

Attachment 375607 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:124:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 157 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 375607 [details]
[Patch] WIP

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

New failing tests:
http/tests/inspector/dom/cross-domain-inspected-node-access.html

[EWS Watchlist]

Created attachment 375614 [details]
Archive of layout-test-results from ews100 for mac-highsierra

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

[Saam Barati]

Comment on attachment 375607 [details]
[Patch] WIP

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

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:769
> +    void appendNode(JSCell*) { }
> +    void appendPropertyNameEdge(JSCell* /* from */, JSCell* /* to */, UniquedStringImpl*) { }
> +    void appendVariableNameEdge(JSCell* /* from */, JSCell* /* to */, UniquedStringImpl*) { }
> +    void appendIndexEdge(JSCell* /* from */, JSCell* /* to */, uint32_t) { }
> +    void setOpaqueRootReachabilityReasonForCell(JSCell*, const char*) { }
> +    void setWrappedObjectForCell(JSCell*, void*) { }
> +    void setLabelForCell(JSCell*, const String&) { }

why do these do nothing? Is the above appendEdge guaranteed to be called too? Why not just not implement them?

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:790
> +    PreventCollectionScope preventCollectionScope(vm.heap);

I don't think this is doing exactly what you want. See below.

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:797
> +        result->putDirectIndex(exec, result->length(), retainer);

In theory this can cause a collection, leading to the HashSet<JSCell*> pointing to things that will get collected.

I'd probably make do something where you run the first half of this as you do now, and then run this loop under DeferGC. Alternatively, you can transfer from a HashSet<JSCell*> to a MarkedArgumentBuffer and just iterate that.

[Devin Rousso]

Comment on attachment 375607 [details]
[Patch] WIP

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

>> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:769
>> +    void setLabelForCell(JSCell*, const String&) { }
> 
> why do these do nothing? Is the above appendEdge guaranteed to be called too? Why not just not implement them?

I had originally thought that `appendEdge` (and also `appendNode`) were guaranteed to be called for every visited edge/node, and that the rest of the functions all get called during each object's `analyzeHeap` (formerly known as `buildSnapshot`) which happens after the GC finishes (`Heap::didFinishCollection`, which calls `Heap::gatherExtraHeapData`, which iterates the `m_objectSpace.forEachLiveCell`).

I now see that both are necessary, so I will add actual code to each of them.

>> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:797
>> +        result->putDirectIndex(exec, result->length(), retainer);
> 
> In theory this can cause a collection, leading to the HashSet<JSCell*> pointing to things that will get collected.
> 
> I'd probably make do something where you run the first half of this as you do now, and then run this loop under DeferGC. Alternatively, you can transfer from a HashSet<JSCell*> to a MarkedArgumentBuffer and just iterate that.

It seems like the other inspector heap related functions all lock the VM (which I don't think is necessary here, as this is a call coming from JavaScript, whereas other inspector things are coming from the inspector frontend) and `DeferGC` from the beginning, so I'll match that too.  Thanks for the head's up!

[Devin Rousso]

Created attachment 375708 [details]
[Patch] WIP

[EWS Watchlist]

Attachment 375708 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:124:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 159 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Joseph Pecoraro]

Comment on attachment 375708 [details]
[Patch] WIP

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

Nice. Makes sense and looks good to me! I like the HeapAnalyzer rename.

We could use a similar approach to mark public (non-inspector) objects in a normal HeapSnapshot so we can hide InjectedScriptSource (or display it differently) in the snapshot viewer.

Currently this could show internal objects that reference public objects like:

    promiseCapability.@promise = promise

I don't think this is a problem, but it is something to consider. We avoided this in queryObjects by just not returning Objects/Arrays.

> Source/JavaScriptCore/heap/HeapAnalyzer.h:45
> +    // A root or marked cell.
> +    virtual void analyzeNode(JSCell*) = 0;
> +
> +    // A reference from one cell to another.
> +    virtual void analyzeEdge(JSCell* from, JSCell* to, SlotVisitor::RootMarkReason) = 0;
> +    virtual void analyzePropertyNameEdge(JSCell* from, JSCell* to, UniquedStringImpl* propertyName) = 0;
> +    virtual void analyzeVariableNameEdge(JSCell* from, JSCell* to, UniquedStringImpl* variableName) = 0;
> +    virtual void analyzeIndexEdge(JSCell* from, JSCell* to, uint32_t index) = 0;

I don't particularly like these being renaming to `analyzeNode` it is more like `visitNode` / `includeEdge` but I guess its not that bad.

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:761
> +        HashSet<JSCell*> queued;

We should really have a comment at this point about the remaining code.

    // Filter the list of retainers down to those reachable from non-Debugger roots.

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:813
> +    HashSet<JSCell*> retainers;

Why isn't this a private `m_retainers` with a getter?

[Devin Rousso]

Created attachment 375761 [details]
[Patch] WIP

[EWS Watchlist]

Attachment 375761 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:124:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 159 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Devin Rousso]

Created attachment 375789 [details]
Patch

[Devin Rousso]

Comment on attachment 375789 [details]
Patch

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

> LayoutTests/inspector/console/queryHolders.html:58
> +                    InspectorTest.expectEqual(remoteObject.type, "object", "The result should be an object.");
> +                    InspectorTest.expectEqual(remoteObject.subtype, "array", "The result should be an array object.");

These could probably just be `InspectorTest.assert` :/

> LayoutTests/inspector/console/queryHolders.html:127
> +                InspectorTest.expectEqual(remoteObject.type, "object", "The result should be an object.");
> +                InspectorTest.expectEqual(remoteObject.subtype, "array", "The result should be an array object.");

Ditto.

[EWS Watchlist]

Attachment 375789 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:124:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 165 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 375789 [details]
Patch

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

New failing tests:
inspector/console/queryHolders.html

[EWS Watchlist]

Created attachment 375797 [details]
Archive of layout-test-results from ews113 for mac-highsierra

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

[Devin Rousso]

Created attachment 376098 [details]
Patch

Skip inspector/console/queryHolders.html for WK1 as heap snapshots are skewed since the inspector shares the same process as the inspected page.

[EWS Watchlist]

Attachment 376098 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:127:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 165 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 376098 [details]
Patch

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

New failing tests:
http/tests/inspector/dom/cross-domain-inspected-node-access.html

[EWS Watchlist]

Created attachment 376102 [details]
Archive of layout-test-results from ews102 for mac-highsierra

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

[EWS Watchlist]

Comment on attachment 376098 [details]
Patch

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

New failing tests:
http/tests/inspector/dom/cross-domain-inspected-node-access.html

[EWS Watchlist]

Created attachment 376110 [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

[EWS Watchlist]

Comment on attachment 376098 [details]
Patch

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

New failing tests:
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-baseline
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla

[Devin Rousso]

Created attachment 376117 [details]
Patch

[EWS Watchlist]

Attachment 376117 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:127:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 166 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Devin Rousso]

Created attachment 376118 [details]
Patch

Remove unnecessary logs in test.

[Devin Rousso]

Comment on attachment 376118 [details]
Patch

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

> LayoutTests/inspector/console/queryHolders-expected.txt:24
> +[Root, Symbol]

I wonder what this `Symbol` is?  I'd expect that the `Map` would also hold on to the `MapKey`, so maybe this is related to that?  If so, perhaps we should reach "up"/"back" one edge if the direct holder isn't an `Object` (e.g. the holders of the holder)?

[EWS Watchlist]

Attachment 376118 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:127:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 166 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 376118 [details]
Patch

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

New failing tests:
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla

[Saam Barati]

Comment on attachment 376118 [details]
Patch

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

The JSC part looks good to me!

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:755
> +    {

this algorithm looks good to me.

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:771
> +            if (!visited.add(from))

nit: I kinda like to query "isNewEntry" just for readability.

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:782
> +    HashSet<JSCell*>&& releaseHolders() { return WTFMove(m_holders); }

nit: the way you're using this, I don't think you're ever actually releasing it, since it doesn't get moved into a copy or constructor. If you really do wanna "release" it, you can just return HashSet<JSCell*> instead of HashSet<JSCell*>&&.

However, in the context you're using it, you can just copy from a reference to this, since it's not important where you "release" it, since the only use is copying to that vector.

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:784
> +    void analyzeEdge(JSCell* from, JSCell* to, SlotVisitor::RootMarkReason reason)

what does this do if from == to? I'm curious what queryHolders returns if an object points to itself.

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:789
> +        std::lock_guard<Lock> lock(m_mutex);

nit: "auto locker = holdLock(m_mutex);"

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:834
> +    if (exec->argumentCount() < 1)
> +        return jsUndefined();
> +
> +    VM& vm = exec->vm();
> +    auto scope = DECLARE_THROW_SCOPE(vm);
> +
> +    JSValue target = exec->uncheckedArgument(0);
> +    if (!target.isObject())
> +        return throwTypeError(exec, scope, "queryHolders first argument must be an object."_s);

why no type error if no argument is provided? Seems equally weird to providing a non-object.

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:839
> +    {

this looks solid to me.

[Devin Rousso]

Comment on attachment 376118 [details]
Patch

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

>> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:782
>> +    HashSet<JSCell*>&& releaseHolders() { return WTFMove(m_holders); }
> 
> nit: the way you're using this, I don't think you're ever actually releasing it, since it doesn't get moved into a copy or constructor. If you really do wanna "release" it, you can just return HashSet<JSCell*> instead of HashSet<JSCell*>&&.
> 
> However, in the context you're using it, you can just copy from a reference to this, since it's not important where you "release" it, since the only use is copying to that vector.

That's true.  I usually like to be more "ideal" than what my code actually needs (e.g. once the holders are computed, the finder shouldn't have any reason to keep hold of the list), but there's no real reason for that.  I'll change it.

>> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:784
>> +    void analyzeEdge(JSCell* from, JSCell* to, SlotVisitor::RootMarkReason reason)
> 
> what does this do if from == to? I'm curious what queryHolders returns if an object points to itself.

I tried adding a `Root.Root = Root` line in the test file and call `queryHolders(Root)`, which just returned `Root`.  I think I should ignore `from == to` edges, as that isn't the "type" of holder that I think most would expect to be returned by `queryHolders` (e.g. a self reference doesn't "really" hold anything strongly).

>> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:834
>> +        return throwTypeError(exec, scope, "queryHolders first argument must be an object."_s);
> 
> why no type error if no argument is provided? Seems equally weird to providing a non-object.

This is typical of `console` and Inspector Console API functions.  We try to be as "accepting" as possible, so no arguments acts like you did nothing (this is how most of the `console.*` functions work too).  The only time we throw exceptions is if we're given something that's completely incompatible (e.g. argument isn't an object).

[Devin Rousso]

Created attachment 376464 [details]
Patch

[EWS Watchlist]

Attachment 376464 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:127:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:778:  Extra space before [.  [whitespace/brackets] [5]
Total errors found: 2 in 166 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 376464 [details]
Patch

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

New failing tests:
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-no-ftl

[Joseph Pecoraro]

> 
> >> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:834
> >> +        return throwTypeError(exec, scope, "queryHolders first argument must be an object."_s);
> > 
> > why no type error if no argument is provided? Seems equally weird to providing a non-object.
> 
> This is typical of `console` and Inspector Console API functions.  We try to
> be as "accepting" as possible, so no arguments acts like you did nothing
> (this is how most of the `console.*` functions work too).  The only time we
> throw exceptions is if we're given something that's completely incompatible
> (e.g. argument isn't an object).

This is true of `console` APIs.

However for Command Line APIs we should probably be as strict as possible. They are only ever reachable when Web Inspector is open and a developer is therefore sitting right there using it. Errors can help them use the Command Line APIs correctly.

[Joseph Pecoraro]

Comment on attachment 376464 [details]
Patch

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

r=me

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:808
> +        if (reason == SlotVisitor::RootMarkReason::Debugger)

Would be great to apply this to HeapSnapshotBuilder and make it so the Web Inspector UI can hide debugger objects!

> Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:335
>      "queryInstances",
>      "queryObjects",
> +    "queryHolders",

Look like this was in alphabetical order. Might as well keep it that way!

> LayoutTests/inspector/console/queryHolders-expected.txt:66
> +-- Running test case: CommandLineAPI.queryHolders.OnlyHeldByDebugger
> +PASS: The result should have 0 items.

I wonder if this will confuse anyone. I think an empty array is the right thing to do though.

> LayoutTests/inspector/console/queryHolders.html:97
> +    addTest(`PromiseFinally`);

Great tests!

Possible additional tests (not required):

  • An object only referenced via an Object Symbol property (should just be like an object string property)
  • An object with a lot of references, like `window` or something like that (ensure nothing crashes)
  • An object referenced in an array a lot of times only shows up once `arr = [foo, foo]`, foo has a single `arr` holder, since it is de-duplicated.

What happens with:

  • Elements. With `$0` do we see the parent element? I'd suspect it might just be the DOMWrapperWorld, which has no good output.

> LayoutTests/inspector/console/queryHolders.html:151
> +    suite.addTestCase({
> +        name: "CommandLineAPI.queryHolders.NoParameter",

Yeah, I kinda think `queryHolders()` and `queryObjects()` no parameters should throw an exception. Could be done as a follow-up. I realize that doesn't match what Chrome does.

[Devin Rousso]

Comment on attachment 376464 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Controllers/JavaScriptRuntimeCompletionProvider.js:335
>> +    "queryHolders",
> 
> Look like this was in alphabetical order. Might as well keep it that way!

Ah oops!  Bad manual rebase merging on my part  :P

>> LayoutTests/inspector/console/queryHolders.html:97
>> +    addTest(`PromiseFinally`);
> 
> Great tests!
> 
> Possible additional tests (not required):
> 
>   • An object only referenced via an Object Symbol property (should just be like an object string property)
>   • An object with a lot of references, like `window` or something like that (ensure nothing crashes)
>   • An object referenced in an array a lot of times only shows up once `arr = [foo, foo]`, foo has a single `arr` holder, since it is de-duplicated.
> 
> What happens with:
> 
>   • Elements. With `$0` do we see the parent element? I'd suspect it might just be the DOMWrapperWorld, which has no good output.

I added three more tests:
 - object held via a `Symbol`
 - create 1000 different objects and have all of them reference the same object
 - create an object with 1000 references to the same object
All three tests behave as you'd expect (object with symbol property is shown, 1000 objects are shown, one object is shown).

As far as `$0` (or any node), the parent-child relationship doesn't appear to "carry over" to JS world.  `queryHolders($0)` actually returns `[]` (unless the underlying `$0` value is also referenced by a JS value).  This is true for any DOM node, though, and that somewhat makes sense, as the parent-child relationship is held by data inside WebCore, not JSC.  Making a DOM node referenced by a parent in any other way than parent-child (e.g. `parent.__foo = child`) _will_ cause the parent to appear in the result.

[Devin Rousso]

Created attachment 376747 [details]
Patch

[EWS Watchlist]

Attachment 376747 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:127:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:778:  Extra space before [.  [whitespace/brackets] [5]
Total errors found: 2 in 166 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Devin Rousso]

Comment on attachment 376464 [details]
Patch

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

>> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:808
>> +        if (reason == SlotVisitor::RootMarkReason::Debugger)
> 
> Would be great to apply this to HeapSnapshotBuilder and make it so the Web Inspector UI can hide debugger objects!

😁 <https://webkit.org/b/200912> Web Inspector: Heap: hide injected inspector objects from snapshots

[Joseph Pecoraro]

> As far as `$0` (or any node), the parent-child relationship doesn't appear
> to "carry over" to JS world.  `queryHolders($0)` actually returns `[]`

Yeah, I think this would be a perfect opportunity to have an InjectedScriptHost virtual method for WebCore to override.

A possible additional holders would be:

  (1) If the JSValue is a Node, and the Node is attached to the DOM, include the `toJS(node.parent)`.
  (2) If the JSValue is a Node, include the `toJS(node.ownerDocument)`

I am kind of partial to (2). Users can already "Reveal in DOM Tree" for (1).

[Devin Rousso]

Created attachment 376790 [details]
Patch

If a holder isn't an object, we should keep looking at successive predecessors (not just the immediate predecessor) for any objects, as there may be other non-object predecessors (e.g. `JSMap` may have multiple levels of `HashMapBucket` before the actual `key` object is reached).

[EWS Watchlist]

Attachment 376790 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:127:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 167 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Devin Rousso]

Created attachment 376815 [details]
Patch

[EWS Watchlist]

Attachment 376815 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/ClassInfo.h:127:  METHOD_TABLE_ENTRY is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 167 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 376815 [details]
Patch

Clearing flags on attachment: 376815

Committed r248925: <https://trac.webkit.org/changeset/248925>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/54536123>