Bug 160325 – Function Call on NULL Object

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jonathan Bedard]

In InjectedBundle.cpp, the 'reset' signal is received at the beginning of a test run.  In this reset, all web notifications are removed from the test runner.  However, the first reset message is received before the test runner has been instantiated.

[Jonathan Bedard]

Note that this problem was only revealed with UBSan since removeAllWebNotifications() does not actually need the test runner to be instantiated to function.

Another possible solution is to make this function a static one, but that would pollute the global namespace and may not be desired.

[Jonathan Bedard]

Created attachment 284832 [details]
Patch

[Darin Adler]

(In reply to comment #1)
> Another possible solution is to make this function a static one, but that
> would pollute the global namespace and may not be desired.

We should make it a static member function. That won’t pollute any namespaces at all.

[Jonathan Bedard]

Created attachment 284867 [details]
Patch

[Jonathan Bedard]

I made all functions in this family static, since none of them actually need the object to work.

As a larger architectural question: why aren't more functions in the TestRunner static?  Inspection of the code seems to indicate that most functions in the TestRunner could be declared as static.

[Darin Adler]

(In reply to comment #5)
> As a larger architectural question: why aren't more functions in the
> TestRunner static?  Inspection of the code seems to indicate that most
> functions in the TestRunner could be declared as static.

TestRunner is primarily an object that exposes bindings to be called from JavaScript using the JavaScript testRunner object exposed to tests. People writing these functions probably don’t think about which ones do and don’t need the state; it would be fine, although not particularly important, to change any that can be static member functions to be defined that way.

[WebKit Commit Bot]

Comment on attachment 284867 [details]
Patch

Clearing flags on attachment: 284867

Committed r203975: <http://trac.webkit.org/changeset/203975>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.