Attachment 368737 Details for Bug 197439 – Patch for landing

[patch] Patch for landing

bug-197439-20190501170303.patch (text/plain), 7.72 KB, created by Ross Kirsling on 2019-05-01 17:03:05 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ross Kirsling

Created: 2019-05-01 17:03:05 PDT

Size: 7.72 KB

patch

obsolete

>Subversion Revision: 244833
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index c54bf38ce688b41366d8f6680277263c3c267116..395fa640aa8249b0e1495a4de19e953c7c0ae63e 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-05-01  Ross Kirsling  <ross.kirsling@sony.com>
>+
>+        RemoteInspector::updateAutomaticInspectionCandidate should have a default implementation.
>+        https://bugs.webkit.org/show_bug.cgi?id=197439
>+
>+        Reviewed by Devin Rousso.
>+
>+        On non-Cocoa platforms, automatic inspection is not currently implemented,
>+        so updateAutomaticInspectionCandidate falls back to the logic of updateTarget.
>+        This logic already existed in three places, so refactor it into a common private method
>+        and allow our websocket-based RWI implementation to make use of it too.
>+
>+        * inspector/remote/RemoteInspector.cpp:
>+        (Inspector::RemoteInspector::updateTarget):
>+        (Inspector::RemoteInspector::updateTargetMap):
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate):
>+        * inspector/remote/RemoteInspector.h:
>+        * inspector/remote/cocoa/RemoteInspectorCocoa.mm:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate):
>+        * inspector/remote/glib/RemoteInspectorGlib.cpp:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate): Deleted.
>+        * inspector/remote/socket/RemoteInspectorSocket.cpp:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate): Deleted.
>+
> 2019-05-01  Darin Adler  <darin@apple.com>
> 
>         WebKit has too much of its own UTF-8 code and should rely more on ICU's UTF-8 support
>diff --git a/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp b/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp
>index 9a6a96f2906f89eeb796bf23a078142c32251f2f..d37d9f018daaead2402f4a4335aab43abfe36339 100644
>--- a/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp
>@@ -104,14 +104,22 @@ void RemoteInspector::updateTarget(RemoteControllableTarget* target)
> 
>     LockHolder lock(m_mutex);
> 
>+    if (!updateTargetMap(target))
>+        return;
>+
>+    pushListingsSoon();
>+}
>+
>+bool RemoteInspector::updateTargetMap(RemoteControllableTarget* target)
>+{
>+    ASSERT(m_mutex.isLocked());
>+
>     auto targetIdentifier = target->targetIdentifier();
>     if (!targetIdentifier)
>-        return;
>+        return false;
> 
>-    {
>-        auto result = m_targetMap.set(targetIdentifier, target);
>-        ASSERT_UNUSED(result, !result.isNewEntry);
>-    }
>+    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.
>@@ -120,8 +128,16 @@ void RemoteInspector::updateTarget(RemoteControllableTarget* target)
>     else
>         m_targetListingMap.remove(targetIdentifier);
> 
>-    pushListingsSoon();
>+    return true;
>+}
>+
>+#if !PLATFORM(COCOA)
>+void RemoteInspector::updateAutomaticInspectionCandidate(RemoteInspectionTarget* target)
>+{
>+    // FIXME: Implement automatic inspection.
>+    updateTarget(target);
> }
>+#endif
> 
> void RemoteInspector::updateClientCapabilities()
> {
>diff --git a/Source/JavaScriptCore/inspector/remote/RemoteInspector.h b/Source/JavaScriptCore/inspector/remote/RemoteInspector.h
>index 71b7cf4ee577d0bb027ec89ec1b38b7b50146b43..3cf93c31e5d117188ddd582c99ab57ba4dd89f51 100644
>--- a/Source/JavaScriptCore/inspector/remote/RemoteInspector.h
>+++ b/Source/JavaScriptCore/inspector/remote/RemoteInspector.h
>@@ -180,6 +180,8 @@ private:
>     TargetListing listingForInspectionTarget(const RemoteInspectionTarget&) const;
>     TargetListing listingForAutomationTarget(const RemoteAutomationTarget&) const;
> 
>+    bool updateTargetMap(RemoteControllableTarget*);
>+
>     void pushListingsNow();
>     void pushListingsSoon();
> 
>diff --git a/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm b/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
>index 11667a8bd7a79414102fddd3223d373b565d49aa..665266b6f5b449a533270af4c54a339fb520c524 100644
>--- a/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
>+++ b/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
>@@ -122,26 +122,17 @@ void RemoteInspector::updateAutomaticInspectionCandidate(RemoteInspectionTarget*
>     {
>         LockHolder lock(m_mutex);
> 
>-        auto targetIdentifier = target->targetIdentifier();
>-        if (!targetIdentifier)
>+        if (!updateTargetMap(target))
>             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;
>         }
> 
>+        auto targetIdentifier = target->targetIdentifier();
>+
>         // 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.
>diff --git a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>index 5aff8b3e3fd7b60d2c4c6e98bc050fa73bf90417..d771ff72afcbc550c86679de21d3a709436511e3 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 b431df0a7f4c23aa864e8c7ce62655e15b73d814..536d3c70a0b2e0f7dc4237bcf0f0bead32f5e0ec 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