Attachment 368635 Details for Bug 197439 – PATCH

[patch] PATCH

197439.diff (text/plain), 6.12 KB, created by Basuke Suzuki on 2019-04-30 17:41:15 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2019-04-30 17:41:15 PDT

Size: 6.12 KB

patch

obsolete

>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 38ccae2b96e..bd37b8603e2 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,23 @@
>+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!).
>+
>+        Implement `updateAutomaticInspectionCandidate()` using refactored method `updateTargetInternal()`.
>+
>+        * inspector/remote/RemoteInspector.cpp:
>+        (Inspector::RemoteInspector::updateTarget):
>+        (Inspector::RemoteInspector::updateTargetInternal): Extracted from updateTarget for shared use.
>+        * inspector/remote/RemoteInspector.h:
>+        * inspector/remote/cocoa/RemoteInspectorCocoa.mm:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate):
>+        * inspector/remote/glib/RemoteInspectorGlib.cpp:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate):
>+        * inspector/remote/socket/RemoteInspectorSocket.cpp:
>+        (Inspector::RemoteInspector::updateAutomaticInspectionCandidate):
>+
> 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..b56e8c1d67f 100644
>--- a/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/RemoteInspector.cpp
>@@ -104,9 +104,17 @@ void RemoteInspector::updateTarget(RemoteControllableTarget* target)
> 
>     LockHolder lock(m_mutex);
> 
>+    if (updateTargetInternal())
>+        pushListingsSoon();
>+}
>+
>+bool RemoteInspector::updateTargetInternal(RemoteControllableTarget* target)
>+{
>+    ASSERT(m_mutex.isLocked());
>+
>     auto targetIdentifier = target->targetIdentifier();
>     if (!targetIdentifier)
>-        return;
>+        return false;
> 
>     {
>         auto result = m_targetMap.set(targetIdentifier, target);
>@@ -120,7 +128,7 @@ void RemoteInspector::updateTarget(RemoteControllableTarget* target)
>     else
>         m_targetListingMap.remove(targetIdentifier);
> 
>-    pushListingsSoon();
>+    return true;
> }
> 
> void RemoteInspector::updateClientCapabilities()
>diff --git a/Source/JavaScriptCore/inspector/remote/RemoteInspector.h b/Source/JavaScriptCore/inspector/remote/RemoteInspector.h
>index 71b7cf4ee57..45c30e024d8 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 updateTargetInternal(RemoteControllableTarget*);
>+
>     void pushListingsNow();
>     void pushListingsSoon();
> 
>diff --git a/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm b/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
>index 5bef21cbb39..8cec7fb2d9c 100644
>--- a/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
>+++ b/Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm
>@@ -122,20 +122,9 @@ void RemoteInspector::updateAutomaticInspectionCandidate(RemoteInspectionTarget*
>     {
>         LockHolder lock(m_mutex);
> 
>-        auto targetIdentifier = target->targetIdentifier();
>-        if (!targetIdentifier)
>+        if (!updateTargetInternal(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();
>diff --git a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>index 5aff8b3e3fd..5049cddee3f 100644
>--- a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>@@ -244,20 +244,9 @@ void RemoteInspector::updateAutomaticInspectionCandidate(RemoteInspectionTarget*
> {
>     LockHolder lock(m_mutex);
> 
>-    ASSERT(target);
>-    auto targetIdentifier = target->targetIdentifier();
>-    if (!targetIdentifier)
>+    if (!updateTargetInternal(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 (auto targetListing = listingForTarget(*target))
>-        m_targetListingMap.set(targetIdentifier, targetListing);
>-    else
>-        m_targetListingMap.remove(targetIdentifier);
>     // FIXME: Implement automatic inspection.
>     pushListingsSoon();
> }
>diff --git a/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp b/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp
>index b431df0a7f4..7b1e4fe530e 100644
>--- a/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp
>@@ -186,6 +186,13 @@ void RemoteInspector::pushListingsSoon()
> 
> void RemoteInspector::updateAutomaticInspectionCandidate(RemoteInspectionTarget*)
> {
>+    LockHolder lock(m_mutex);
>+
>+    if (!updateTargetInternal(target))
>+        return;
>+
>+    // FIXME: Implement automatic inspection.
>+    pushListingsSoon();
> }
> 
> void RemoteInspector::sendAutomaticInspectionCandidateMessage()

Actions: View | Formatted Diff | Diff

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