Bug 221108 – check-webkit-style: warn about WTF::BlockPtr use in JavaScriptCore until ARC is enabled

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

check-webkit-style: warn about WTF::BlockPtr use in JavaScriptCore until ARC is enabled.

This is to catch bugs like this from occurring before code is committed:

    Bug 220956: Crash when remote inspecting in debug builds
    <https://bugs.webkit.org/show_bug.cgi?id=220956>

See also:

    Bug 221107: JavaScriptCore should build with ARC enabled
    <https://bugs.webkit.org/show_bug.cgi?id=221107>

[Radar WebKit Bug Importer]

<rdar://problem/73726640>

[David Kilzer (:ddkilzer)]

This issue was introduced when WTF was converted to ARC since WTF::BlockPtr existed at that time:

    Bug 186961: [Cocoa] Convert the small bit of Objective-C++ code in WTF to ARC
    <https://bugs.webkit.org/show_bug.cgi?id=186961>

    Bug 152799: Add a smart block pointer
    <https://bugs.webkit.org/show_bug.cgi?id=152799>

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #2)
> This issue was introduced when WTF was converted to ARC since WTF::BlockPtr
> existed at that time:

Or rather, it became possible for this issue to exist once this happened, but we only know of one bug (Bug 220956) that was actually caused by this.

[David Kilzer (:ddkilzer)]

Created attachment 418702 [details]
Patch v1

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #4)
> Created attachment 418702 [details]
> Patch v1

Test failures are not related to this patch (missing results, or these *.xml files should be skipped since they're not tests.

[David Kilzer (:ddkilzer)]

Created attachment 418729 [details]
Patch v2

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #6)
> Created attachment 418729 [details]
> Patch v2

Made the regex check for "BlockPtr<" a little more robust.

[Joseph Pecoraro]

Comment on attachment 418729 [details]
Patch v2

Nice! r=me

[David Kilzer (:ddkilzer)]

The webkitpy failure is unrelated to this change (I think this Python test is flaky):

[41/1961] webkitcorepy.tests.task_pool_unittest.TaskPoolUnittest.test_invalid_shutdown failed:
  Traceback (most recent call last):
    File "/Volumes/Data/worker/WebKitPy-Tests-EWS/build/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/tests/task_pool_unittest.py", line 136, in test_invalid_shutdown
      pool.do(wait, 2)
  AssertionError: Exception not raised

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #9)
> The webkitpy failure is unrelated to this change (I think this Python test
> is flaky):
> 
> [41/1961]
> webkitcorepy.tests.task_pool_unittest.TaskPoolUnittest.test_invalid_shutdown
> failed:
>   Traceback (most recent call last):
>     File
> "/Volumes/Data/worker/WebKitPy-Tests-EWS/build/Tools/Scripts/libraries/
> webkitcorepy/webkitcorepy/tests/task_pool_unittest.py", line 136, in
> test_invalid_shutdown
>       pool.do(wait, 2)
>   AssertionError: Exception not raised

<https://ews-build.webkit.org/#/builders/5/builds/42925>

[EWS]

Committed r272064: <https://trac.webkit.org/changeset/272064>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418729 [details].