WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
212312
[PlayStation] Enable RemoteWebInspector
https://bugs.webkit.org/show_bug.cgi?id=212312
Summary
[PlayStation] Enable RemoteWebInspector
Yoshiaki Jitsukawa
Reported
2020-05-24 01:10:30 PDT
Enable RemoteWebInspector on PlayStation.
Attachments
Patch
(8.30 KB, patch)
2020-05-24 01:21 PDT
,
Yoshiaki Jitsukawa
no flags
Details
Formatted Diff
Diff
Patch
(5.63 KB, patch)
2020-05-26 13:50 PDT
,
Yoshiaki Jitsukawa
no flags
Details
Formatted Diff
Diff
Patch
(3.57 KB, patch)
2020-05-26 14:06 PDT
,
Yoshiaki Jitsukawa
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Yoshiaki Jitsukawa
Comment 1
2020-05-24 01:21:16 PDT
Created
attachment 400151
[details]
Patch
Don Olmstead
Comment 2
2020-05-26 08:37:55 PDT
Comment on
attachment 400151
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=400151&action=review
> Source/JavaScriptCore/API/JSRemoteInspectorServer.cpp:33 > + auto& server = Inspector::RemoteInspectorServer::singleton();
Any reason this should change?
> Source/JavaScriptCore/PlatformPlayStation.cmake:2 > + API/JSBasePrivate.h
I know why you have this here since there's that unhandled promise handler rejection handler that never became a public C API but its still a PRIVATE_FRAMEWORK_HEADER. Add it to the list of headers to install.
> Source/WTF/wtf/PlatformEnable.h:100 > +#include <wtf/PlatformEnablePlayStation.h>
I feel like this is a separate change unrelated to this bug.
> Source/cmake/OptionsPlayStation.cmake:35 > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_REMOTE_INSPECTOR PRIVATE ON)
You need to remove ENABLE_REMOTE_INSPECTOR from the Experimental features listing below.
Devin Rousso
Comment 3
2020-05-26 08:47:37 PDT
Comment on
attachment 400151
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=400151&action=review
> Source/JavaScriptCore/inspector/remote/socket/posix/RemoteInspectorSocketPOSIX.cpp:93 > }
Style: please add a newline after multiline `if` that have a `return`
Yoshiaki Jitsukawa
Comment 4
2020-05-26 13:40:52 PDT
(In reply to Don Olmstead from
comment #2
)
> Comment on
attachment 400151
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=400151&action=review
> > > Source/JavaScriptCore/API/JSRemoteInspectorServer.cpp:33 > > + auto& server = Inspector::RemoteInspectorServer::singleton(); > > Any reason this should change?
That's because start() is not a const function and it doesn't compile as-is.
> > Source/JavaScriptCore/PlatformPlayStation.cmake:2 > > + API/JSBasePrivate.h > > I know why you have this here since there's that unhandled promise handler > rejection handler that never became a public C API but its still a > PRIVATE_FRAMEWORK_HEADER. Add it to the list of headers to install.
Ok,JSBasePrivate.h itself is not needed with this patch. I'll remove this line.
> > Source/WTF/wtf/PlatformEnable.h:100 > > +#include <wtf/PlatformEnablePlayStation.h> > > I feel like this is a separate change unrelated to this bug.
Yes, will do.
> > Source/cmake/OptionsPlayStation.cmake:35 > > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_REMOTE_INSPECTOR PRIVATE ON) > > You need to remove ENABLE_REMOTE_INSPECTOR from the Experimental features > listing below.
Oh, I see. (In reply to Devin Rousso from
comment #3
)
> Comment on
attachment 400151
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=400151&action=review
> > > Source/JavaScriptCore/inspector/remote/socket/posix/RemoteInspectorSocketPOSIX.cpp:93 > > } > > Style: please add a newline after multiline `if` that have a `return`
Sure.
Yoshiaki Jitsukawa
Comment 5
2020-05-26 13:50:11 PDT
Created
attachment 400271
[details]
Patch
Yoshiaki Jitsukawa
Comment 6
2020-05-26 14:06:19 PDT
Created
attachment 400274
[details]
Patch
Don Olmstead
Comment 7
2020-05-26 15:33:27 PDT
Comment on
attachment 400274
[details]
Patch r=me Didn’t you want to enable remote inspector by default? My comment was that to do that you needed to remove the line tying it to experimental features as well
Yoshiaki Jitsukawa
Comment 8
2020-05-26 15:39:56 PDT
(In reply to Don Olmstead from
comment #7
)
> Comment on
attachment 400274
[details]
> Patch > > r=me > > Didn’t you want to enable remote inspector by default? My comment was that > to do that you needed to remove the line tying it to experimental features > as well
I did, but now I don't because it may make it a bit difficult to land WebKit portion so let me keep the default value as it is for now.
EWS
Comment 9
2020-05-26 15:43:45 PDT
Committed
r262166
: <
https://trac.webkit.org/changeset/262166
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 400274
[details]
.
Radar WebKit Bug Importer
Comment 10
2020-05-26 15:44:14 PDT
<
rdar://problem/63646513
>
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