Attachment 368628 Details for Bug 197439 – PATCH

[patch] PATCH

197439.diff (text/plain), 10.40 KB, created by Basuke Suzuki on 2019-04-30 16:39:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2019-04-30 16:39:13 PDT

Size: 10.40 KB

patch

obsolete

>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 38ccae2b96e..c7b2ee7cf94 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-04-30  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [PlayStation] targetListingMap is not update when RemoteInspector has started after target.
>+        https://bugs.webkit.org/show_bug.cgi?id=197439
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When RemoteInspector starts after targets, this method is called to update targetListingMap.
>+        All implementation do almost identical. Move it to shared implementation file.
>+
>+        * inspector/remote/RemoteInspector.cpp:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate):
>+        * inspector/remote/cocoa/RemoteInspectorCocoa.mm:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate): Moved to RemoteInspector.cpp.
>+        * inspector/remote/glib/RemoteInspectorGlib.cpp:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate): Deleted.
>+        * inspector/remote/socket/RemoteInspectorSocket.cpp:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate): Deleted.
>+
> 2019-04-29  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         JITStubRoutineSet wastes 180KB of HashTable capacity on can.com
>diff --git a/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp b/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp
>index 9a6a96f2906..cf8b5d978dc 100644
>--- a/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp
>@@ -240,6 +240,80 @@ void RemoteInspector::updateHasActiveDebugSession()
>     // Legacy iOS WebKit 1 had a notification. This will need to be smarter with WebKit2.
> }
> 
>+void RemoteInspector::updateAutomaticInspectionCandidate(RemoteInspectionTarget* target)
>+{
>+    ASSERT_ARG(target, target);
>+    {
>+        LockHolder lock(m_mutex);
>+
>+        auto targetIdentifier = target->targetIdentifier();
>+        if (!targetIdentifier)
>+            return;
>+
>+        auto result = m_targetMap.set(targetIdentifier, target);
>+        ASSERT_UNUSED(result, !result.isNewEntry);
>+
>+        // If the target has just allowed remote control, then the listing won't exist yet.
>+        // If the target has no identifier remove the old listing.
>+        if (auto targetListing = listingForTarget(*target))
>+            m_targetListingMap.set(targetIdentifier, targetListing);
>+        else
>+            m_targetListingMap.remove(targetIdentifier);
>+
>+#if USE(GLIB) || PLATFORM(PLAYSTATION)
>+        // FIXME: Implement automatic inspection.
>+        pushListingsSoon();
>+#else
>+        // Don't allow automatic inspection unless it is allowed or we are stopped.
>+        if (!m_automaticInspectionEnabled || !m_enabled) {
>+            pushListingsSoon();
>+            return;
>+        }
>+
>+        // FIXME: We should handle multiple debuggables trying to pause at the same time on different threads.
>+        // To make this work we will need to change m_automaticInspectionCandidateTargetIdentifier to be a per-thread value.
>+        // Multiple attempts on the same thread should not be possible because our nested run loop is in a special RWI mode.
>+        if (m_automaticInspectionPaused) {
>+            LOG_ERROR("Skipping Automatic Inspection Candidate with pageId(%u) because we are already paused waiting for pageId(%u)", targetIdentifier, m_automaticInspectionCandidateTargetIdentifier);
>+            pushListingsSoon();
>+            return;
>+        }
>+
>+        m_automaticInspectionPaused = true;
>+        m_automaticInspectionCandidateTargetIdentifier = targetIdentifier;
>+
>+        // If we are pausing before we have connected to webinspectord the candidate message will be sent as soon as the connection is established.
>+        if (m_relayConnection) {
>+            pushListingsNow();
>+            sendAutomaticInspectionCandidateMessage();
>+        }
>+
>+        // In case debuggers fail to respond, or we cannot connect to webinspectord, automatically continue after a short period of time.
>+#if PLATFORM(WATCHOS)
>+        int64_t debuggerTimeoutDelay = 5;
>+#else
>+        int64_t debuggerTimeoutDelay = 1;
>+#endif
>+        dispatch_after(dispatch_time(DISPATCH_TIME_NOW, debuggerTimeoutDelay * NSEC_PER_SEC), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
>+            LockHolder lock(m_mutex);
>+            if (m_automaticInspectionCandidateTargetIdentifier == targetIdentifier) {
>+                LOG_ERROR("Skipping Automatic Inspection Candidate with pageId(%u) because we failed to receive a response in time.", m_automaticInspectionCandidateTargetIdentifier);
>+                m_automaticInspectionPaused = false;
>+            }
>+        });
>+#endif
>+    }
>+
>+    target->pauseWaitingForAutomaticInspection();
>+
>+    {
>+        LockHolder lock(m_mutex);
>+
>+        ASSERT(m_automaticInspectionCandidateTargetIdentifier);
>+        m_automaticInspectionCandidateTargetIdentifier = 0;
>+    }
>+}
>+
> RemoteInspector::Client::~Client()
> {
> }
>diff --git a/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm b/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
>index 5bef21cbb39..8b67187f6bc 100644
>--- a/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
>+++ b/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
>@@ -116,75 +116,6 @@ RemoteInspector::RemoteInspector()
> {
> }
> 
>-void RemoteInspector::updateAutomaticInspectionCandidate(RemoteInspectionTarget* target)
>-{
>-    ASSERT_ARG(target, target);
>-    {
>-        LockHolder lock(m_mutex);
>-
>-        auto targetIdentifier = target->targetIdentifier();
>-        if (!targetIdentifier)
>-            return;
>-
>-        auto result = m_targetMap.set(targetIdentifier, target);
>-        ASSERT_UNUSED(result, !result.isNewEntry);
>-
>-        // If the target has just allowed remote control, then the listing won't exist yet.
>-        // If the target has no identifier remove the old listing.
>-        if (RetainPtr<NSDictionary> targetListing = listingForTarget(*target))
>-            m_targetListingMap.set(targetIdentifier, targetListing);
>-        else
>-            m_targetListingMap.remove(targetIdentifier);
>-
>-        // Don't allow automatic inspection unless it is allowed or we are stopped.
>-        if (!m_automaticInspectionEnabled || !m_enabled) {
>-            pushListingsSoon();
>-            return;
>-        }
>-
>-        // FIXME: We should handle multiple debuggables trying to pause at the same time on different threads.
>-        // To make this work we will need to change m_automaticInspectionCandidateTargetIdentifier to be a per-thread value.
>-        // Multiple attempts on the same thread should not be possible because our nested run loop is in a special RWI mode.
>-        if (m_automaticInspectionPaused) {
>-            LOG_ERROR("Skipping Automatic Inspection Candidate with pageId(%u) because we are already paused waiting for pageId(%u)", targetIdentifier, m_automaticInspectionCandidateTargetIdentifier);
>-            pushListingsSoon();
>-            return;
>-        }
>-
>-        m_automaticInspectionPaused = true;
>-        m_automaticInspectionCandidateTargetIdentifier = targetIdentifier;
>-
>-        // If we are pausing before we have connected to webinspectord the candidate message will be sent as soon as the connection is established.
>-        if (m_relayConnection) {
>-            pushListingsNow();
>-            sendAutomaticInspectionCandidateMessage();
>-        }
>-
>-        // In case debuggers fail to respond, or we cannot connect to webinspectord, automatically continue after a short period of time.
>-#if PLATFORM(WATCHOS)
>-        int64_t debuggerTimeoutDelay = 5;
>-#else
>-        int64_t debuggerTimeoutDelay = 1;
>-#endif
>-        dispatch_after(dispatch_time(DISPATCH_TIME_NOW, debuggerTimeoutDelay * NSEC_PER_SEC), dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
>-            LockHolder lock(m_mutex);
>-            if (m_automaticInspectionCandidateTargetIdentifier == targetIdentifier) {
>-                LOG_ERROR("Skipping Automatic Inspection Candidate with pageId(%u) because we failed to receive a response in time.", m_automaticInspectionCandidateTargetIdentifier);
>-                m_automaticInspectionPaused = false;
>-            }
>-        });
>-    }
>-
>-    target->pauseWaitingForAutomaticInspection();
>-
>-    {
>-        LockHolder lock(m_mutex);
>-
>-        ASSERT(m_automaticInspectionCandidateTargetIdentifier);
>-        m_automaticInspectionCandidateTargetIdentifier = 0;
>-    }
>-}
>-
> void RemoteInspector::sendAutomaticInspectionCandidateMessage()
> {
>     ASSERT(m_enabled);
>diff --git a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>index 5aff8b3e3fd..d771ff72afc 100644
>--- a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>@@ -240,28 +240,6 @@ void RemoteInspector::pushListingsSoon()
>     });
> }
> 
>-void RemoteInspector::updateAutomaticInspectionCandidate(RemoteInspectionTarget* target)
>-{
>-    LockHolder lock(m_mutex);
>-
>-    ASSERT(target);
>-    auto targetIdentifier = target->targetIdentifier();
>-    if (!targetIdentifier)
>-        return;
>-
>-    auto result = m_targetMap.set(targetIdentifier, target);
>-    ASSERT_UNUSED(result, !result.isNewEntry);
>-
>-    // If the target has just allowed remote control, then the listing won't exist yet.
>-    // If the target has no identifier remove the old listing.
>-    if (auto targetListing = listingForTarget(*target))
>-        m_targetListingMap.set(targetIdentifier, targetListing);
>-    else
>-        m_targetListingMap.remove(targetIdentifier);
>-    // FIXME: Implement automatic inspection.
>-    pushListingsSoon();
>-}
>-
> void RemoteInspector::sendAutomaticInspectionCandidateMessage()
> {
>     ASSERT(m_enabled);
>diff --git a/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp b/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp
>index b431df0a7f4..536d3c70a0b 100644
>--- a/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp
>@@ -184,10 +184,6 @@ void RemoteInspector::pushListingsSoon()
>     });
> }
> 
>-void RemoteInspector::updateAutomaticInspectionCandidate(RemoteInspectionTarget*)
>-{
>-}
>-
> void RemoteInspector::sendAutomaticInspectionCandidateMessage()
> {
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 197439: 368608 | 368611 | 368628 | 368635 | 368703 | 368721 | 368737