Bug 170770 - [JSC] Clean up heap/MachineStackMarker by introducing HAVE(MACHINE_CONTEXT)
Summary: [JSC] Clean up heap/MachineStackMarker by introducing HAVE(MACHINE_CONTEXT)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-12 07:34 PDT by Yusuke Suzuki
Modified: 2017-04-12 10:29 PDT (History)
10 users (show)

See Also:


Attachments
Patch (15.69 KB, patch)
2017-04-12 07:36 PDT, Yusuke Suzuki
mark.lam: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.05 MB, application/zip)
2017-04-12 09:14 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2017-04-12 07:34:32 PDT
[JSC] Clean up heap/MachineStackMarker by introducing USE(MACHINE_CONTEXT)
Comment 1 Yusuke Suzuki 2017-04-12 07:36:23 PDT
Created attachment 306909 [details]
Patch
Comment 2 Build Bot 2017-04-12 09:13:58 PDT
Comment on attachment 306909 [details]
Patch

Attachment 306909 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3524411

New failing tests:
webrtc/negotiatedneeded-event-addStream.html
Comment 3 Build Bot 2017-04-12 09:14:00 PDT
Created attachment 306914 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 4 Mark Lam 2017-04-12 09:38:58 PDT
Comment on attachment 306909 [details]
Patch

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

r=me with suggestion.

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:235
> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT)

Here, you explicitly  call out OS(DARWIN) even though USE(MACHINE_CONTEXT) already include OS(DARWIN).  Later below, you don't call out OS(DARWIN).  Let's be consistent.  How about we don't call out OS(DARWIN) explicitly?

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:244
> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT)

Ditto.

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:254
> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT)

Ditto.
Comment 5 Yusuke Suzuki 2017-04-12 09:53:58 PDT
Comment on attachment 306909 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/heap/MachineStackMarker.cpp:235
>> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT)
> 
> Here, you explicitly  call out OS(DARWIN) even though USE(MACHINE_CONTEXT) already include OS(DARWIN).  Later below, you don't call out OS(DARWIN).  Let's be consistent.  How about we don't call out OS(DARWIN) explicitly?

OK, let's drop OS(DARWIN) here.

>> Source/JavaScriptCore/heap/MachineStackMarker.cpp:244
>> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT)
> 
> Ditto.

Fixed.

>> Source/JavaScriptCore/heap/MachineStackMarker.cpp:254
>> +#if OS(DARWIN) || OS(WINDOWS) || USE(MACHINE_CONTEXT)
> 
> Ditto.

Fixed.
Comment 6 Yusuke Suzuki 2017-04-12 09:59:39 PDT
Committed r215269: <http://trac.webkit.org/changeset/215269>
Comment 7 Yusuke Suzuki 2017-04-12 10:20:06 PDT
Committed r215270: <http://trac.webkit.org/changeset/215270>