WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
170770
[JSC] Clean up heap/MachineStackMarker by introducing HAVE(MACHINE_CONTEXT)
https://bugs.webkit.org/show_bug.cgi?id=170770
Summary
[JSC] Clean up heap/MachineStackMarker by introducing HAVE(MACHINE_CONTEXT)
Yusuke Suzuki
Reported
2017-04-12 07:34:32 PDT
[JSC] Clean up heap/MachineStackMarker by introducing USE(MACHINE_CONTEXT)
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
View All
Add attachment
proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1
2017-04-12 07:36:23 PDT
Created
attachment 306909
[details]
Patch
Build Bot
Comment 2
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
Build Bot
Comment 3
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
Mark Lam
Comment 4
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.
Yusuke Suzuki
Comment 5
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.
Yusuke Suzuki
Comment 6
2017-04-12 09:59:39 PDT
Committed
r215269
: <
http://trac.webkit.org/changeset/215269
>
Yusuke Suzuki
Comment 7
2017-04-12 10:20:06 PDT
Committed
r215270
: <
http://trac.webkit.org/changeset/215270
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug