The required method RemoteInspector::updateAutomaticInspectionCandidate() was not implemented.
Created attachment 368608 [details] PATCH
Created attachment 368611 [details] PATCH
Looks like this is a copy-paste of the Glib implementation -- unless we have an expectation for the two to diverge, perhaps it should be moved to a common location instead?
(In reply to Ross Kirsling from comment #3) > Looks like this is a copy-paste of the Glib implementation -- unless we have > an expectation for the two to diverge, perhaps it should be moved to a > common location instead? Humm, good point. I didn't think about that before because it was separated before we've start adding our port. It seems almost neutral for all port. I'll try moving it in shared implementation.
Created attachment 368628 [details] PATCH
Created attachment 368635 [details] PATCH
Comment on attachment 368635 [details] PATCH At this moment, this is a good balance to share code. We'll do more when we implement automatic inspection.
(In reply to Basuke Suzuki from comment #7) > Comment on attachment 368635 [details] > PATCH > > At this moment, this is a good balance to share code. We'll do more when we > implement automatic inspection. I'd probably change the bug name at this time since this is touching more than playstation.
Comment on attachment 368635 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=368635&action=review > Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp:107 > + if (updateTargetInternal()) This should have `target` passed in then it should build.
Created attachment 368703 [details] Patch
Created attachment 368721 [details] Patch
Comment on attachment 368721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=368721&action=review r=me > Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp:113 > +bool RemoteInspector::updateTargetInternal(RemoteControllableTarget* target) This is an odd name. How about `updateListingForTarget` or `updateTargetMap`? > Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp:-111 > - { NIT: I realize this block is unnecessary, but I wouldn't change it for the sake of keeping the diff cleaner :P
(In reply to Devin Rousso from comment #12) > Comment on attachment 368721 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=368721&action=review > > r=me > > > Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp:113 > > +bool RemoteInspector::updateTargetInternal(RemoteControllableTarget* target) > > This is an odd name. How about `updateListingForTarget` or > `updateTargetMap`? I almost changed this to `updateTargetListings` but it turns out that `updateTargetListing` is already a method! That one literally just updates m_targetListingMap, whereas this one updates m_targetMap then sets OR clears m_targetListingMap. I suppose `updateTargetMap` could be clearer if that were taken to imply an update to the listing too -- do you think that's the case? > > Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp:-111 > > - { > > NIT: I realize this block is unnecessary, but I wouldn't change it for the > sake of keeping the diff cleaner :P I would have left it, but since the Glib version doesn't have it, I thought it'd be okay to remove it while merging implementations, haha.
Created attachment 368737 [details] Patch for landing
Comment on attachment 368737 [details] Patch for landing Clearing flags on attachment: 368737 Committed r244861: <https://trac.webkit.org/changeset/244861>
All reviewed patches have been landed. Closing bug.
<rdar://problem/50424864>