Changeset 252124 in webkit


Ignore:
Timestamp:
Nov 5, 2019, 10:36:28 PM (5 years ago)
Author:
mark.lam@apple.com
Message:

WTF::RunLoop should not depend on isMainThread() idiom.
​https://bugs.webkit.org/show_bug.cgi?id=203873
<rdar://problem/56524251>

Reviewed by Saam Barati, Ryosuke Niwa, and Devin Rousso.

Source/JavaScriptCore:

  • inspector/JSGlobalObjectScriptDebugServer.cpp:

(Inspector::JSGlobalObjectScriptDebugServer::runLoopMode):

  • inspector/JSGlobalObjectScriptDebugServer.h:
  • inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm:

(Inspector::RemoteTargetInitializeGlobalQueue):
(Inspector::RemoteConnectionToTarget::setupRunLoop):
(Inspector::RemoteConnectionToTarget::teardownRunLoop):

Source/WTF:

The isMainThread() idiom is only meaningful for WebCore. It is less meaningful
for JSC since a VM instance can be entered from multiple threads, as long as only
one thread enters it at any time. Hence, the concept of a main thread doesn't
make sense at the JSC level.

Since r251036, we started using a WTF::String to represent the RunLoop mode.
This caused problems for JSC clients when USE(CF) since it necessitated the use of
StringWrapperCFAllocator to track the life cycle of the CFStringRef generated from
the WTF::String.

To fix this problem, we should restore the original behavior of using CFStringRefs
as the RunLoop mode token.

  • wtf/RunLoop.h:

(WTF::RunLoop::cycle): Deleted.

  • wtf/cf/RunLoopCF.cpp:

(WTF::RunLoop::cycle):

  • wtf/generic/RunLoopGeneric.cpp:

(WTF::RunLoop::cycle):

  • wtf/glib/RunLoopGLib.cpp:

(WTF::RunLoop::cycle):

  • wtf/win/RunLoopWin.cpp:

(WTF::RunLoop::cycle):

Location:
trunk/Source
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r252115 r252124  
     12019-11-05  Mark Lam  <mark.lam@apple.com>
     2
     3        WTF::RunLoop should not depend on isMainThread() idiom.
     4        https://bugs.webkit.org/show_bug.cgi?id=203873
     5        <rdar://problem/56524251>
     6
     7        Reviewed by Saam Barati, Ryosuke Niwa, and Devin Rousso.
     8
     9        * inspector/JSGlobalObjectScriptDebugServer.cpp:
     10        (Inspector::JSGlobalObjectScriptDebugServer::runLoopMode):
     11        * inspector/JSGlobalObjectScriptDebugServer.h:
     12        * inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm:
     13        (Inspector::RemoteTargetInitializeGlobalQueue):
     14        (Inspector::RemoteConnectionToTarget::setupRunLoop):
     15        (Inspector::RemoteConnectionToTarget::teardownRunLoop):
     16
    1172019-11-05  Tadeu Zagallo  <tzagallo@apple.com>
    218

  • trunk/Source/JavaScriptCore/inspector/JSGlobalObjectScriptDebugServer.cpp

    r251036 r252124  
    11/*
    2  * Copyright (C) 2014 Apple Inc. All rights reserved.
     2 * Copyright (C) 2014-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    6565}
    6666
    67 String JSGlobalObjectScriptDebugServer::runLoopMode()
     67RunLoopMode JSGlobalObjectScriptDebugServer::runLoopMode()
    6868{
    6969#if USE(CF) && !PLATFORM(WATCHOS)
    … …  
    7777    // we need to run in the default run loop mode otherwise we do not receive the XPC messages
    7878    // necessary to setup the relay connection and negotiate an auto-attach debugger.
    79     return "com.apple.JavaScriptCore.remote-inspector-runloop-mode"_s;
     79    return CFSTR("com.apple.JavaScriptCore.remote-inspector-runloop-mode");
    8080#else
    81     return { };
     81    return DefaultRunLoopMode;
    8282#endif
    8383}

  • trunk/Source/JavaScriptCore/inspector/JSGlobalObjectScriptDebugServer.h

    r251425 r252124  
    11/*
    2  * Copyright (C) 2014 Apple Inc. All rights reserved.
     2 * Copyright (C) 2014-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2727
    2828#include "ScriptDebugServer.h"
     29#include <wtf/RunLoop.h>
    2930
    3031namespace Inspector {
    … …  
    3940    JSC::JSGlobalObject& globalObject() const { return m_globalObject; }
    4041
    41     static String runLoopMode();
     42    static RunLoopMode runLoopMode();
    4243
    4344private:

  • trunk/Source/JavaScriptCore/inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm

    r251036 r252124  
    11/*
    2  * Copyright (C) 2013, 2015 Apple Inc. All Rights Reserved.
     2 * Copyright (C) 2013-2019 Apple Inc. All Rights Reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    9090        CFRunLoopAddSource(CFRunLoopGetMain(), rwiRunLoopSource, kCFRunLoopDefaultMode);
    9191        auto mode = JSGlobalObjectScriptDebugServer::runLoopMode();
    92         if (!mode.isNull())
    93             CFRunLoopAddSource(CFRunLoopGetMain(), rwiRunLoopSource, mode.createCFString().get());
     92        if (mode != DefaultRunLoopMode)
     93            CFRunLoopAddSource(CFRunLoopGetMain(), rwiRunLoopSource, mode);
    9494    });
    9595}
    … …  
    263263    CFRunLoopAddSource(m_runLoop.get(), m_runLoopSource.get(), kCFRunLoopDefaultMode);
    264264    auto mode = JSGlobalObjectScriptDebugServer::runLoopMode();
    265     if (!mode.isNull())
    266         CFRunLoopAddSource(m_runLoop.get(), m_runLoopSource.get(), mode.createCFString().get());
     265    if (mode != DefaultRunLoopMode)
     266        CFRunLoopAddSource(m_runLoop.get(), m_runLoopSource.get(), mode);
    267267}
    268268
    … …  
    274274    CFRunLoopRemoveSource(m_runLoop.get(), m_runLoopSource.get(), kCFRunLoopDefaultMode);
    275275    auto mode = JSGlobalObjectScriptDebugServer::runLoopMode();
    276     if (!mode.isNull())
    277         CFRunLoopRemoveSource(m_runLoop.get(), m_runLoopSource.get(), mode.createCFString().get());
     276    if (mode != DefaultRunLoopMode)
     277        CFRunLoopRemoveSource(m_runLoop.get(), m_runLoopSource.get(), mode);
    278278
    279279    m_runLoop = nullptr;

  • trunk/Source/WTF/ChangeLog

    r252068 r252124  
     12019-11-05  Mark Lam  <mark.lam@apple.com>
     2
     3        WTF::RunLoop should not depend on isMainThread() idiom.
     4        https://bugs.webkit.org/show_bug.cgi?id=203873
     5        <rdar://problem/56524251>
     6
     7        Reviewed by Saam Barati, Ryosuke Niwa, and Devin Rousso.
     8
     9        The isMainThread() idiom is only meaningful for WebCore.  It is less meaningful
     10        for JSC since a VM instance can be entered from multiple threads, as long as only
     11        one thread enters it at any time.  Hence, the concept of a main thread doesn't
     12        make sense at the JSC level.
     13
     14        Since r251036, we started using a WTF::String to represent the RunLoop mode.
     15        This caused problems for JSC clients when USE(CF) since it necessitated the use of
     16        StringWrapperCFAllocator to track the life cycle of the CFStringRef generated from
     17        the WTF::String.
     18
     19        To fix this problem, we should restore the original behavior of using CFStringRefs
     20        as the RunLoop mode token.
     21
     22        * wtf/RunLoop.h:
     23        (WTF::RunLoop::cycle): Deleted.
     24        * wtf/cf/RunLoopCF.cpp:
     25        (WTF::RunLoop::cycle):
     26        * wtf/generic/RunLoopGeneric.cpp:
     27        (WTF::RunLoop::cycle):
     28        * wtf/glib/RunLoopGLib.cpp:
     29        (WTF::RunLoop::cycle):
     30        * wtf/win/RunLoopWin.cpp:
     31        (WTF::RunLoop::cycle):
     32
    1332019-11-05  Tuomas Karkkainen  <tuomas.webkit@apple.com>
    234

  • trunk/Source/WTF/wtf/RunLoop.h

    r251514 r252124  
    11/*
    2  * Copyright (C) 2010 Apple Inc. All rights reserved.
     2 * Copyright (C) 2010-2019 Apple Inc. All rights reserved.
    33 * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
    44 * Portions Copyright (c) 2010 Motorola Mobility, Inc.  All rights reserved.
    … …  
    3838#include <wtf/text/WTFString.h>
    3939
     40#if USE(CF)
     41#include <CoreFoundation/CFRunLoop.h>
     42#endif
     43
    4044#if USE(GLIB_EVENT_LOOP)
    4145#include <wtf/glib/GRefPtr.h>
    … …  
    4347
    4448namespace WTF {
     49
     50#if USE(CF)
     51using RunLoopMode = CFStringRef;
     52#define DefaultRunLoopMode kCFRunLoopDefaultMode
     53#else
     54using RunLoopMode = unsigned;
     55#define DefaultRunLoopMode 0
     56#endif
    4557
    4658class RunLoop : public FunctionDispatcher {
    … …  
    6375
    6476    enum class CycleResult { Continue, Stop };
    65     WTF_EXPORT_PRIVATE CycleResult static cycle(const String& = { });
     77    WTF_EXPORT_PRIVATE CycleResult static cycle(RunLoopMode = DefaultRunLoopMode);
    6678
    6779#if USE(COCOA_EVENT_LOOP)
    … …  
    219231
    220232using WTF::RunLoop;
     233using WTF::RunLoopMode;

  • trunk/Source/WTF/wtf/cf/RunLoopCF.cpp

    r251514 r252124  
    11/*
    2  * Copyright (C) 2010, 2012 Apple Inc. All rights reserved.
     2 * Copyright (C) 2010-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    6363}
    6464
    65 RunLoop::CycleResult RunLoop::cycle(const String& mode)
     65RunLoop::CycleResult RunLoop::cycle(RunLoopMode mode)
    6666{
    6767    CFTimeInterval timeInterval = 0.05;
    68     CFRunLoopRunInMode(mode.isNull() ? kCFRunLoopDefaultMode : mode.createCFString().get(), timeInterval, true);
     68    CFRunLoopRunInMode(mode, timeInterval, true);
    6969    return CycleResult::Continue;
    7070}

  • trunk/Source/WTF/wtf/generic/RunLoopGeneric.cpp

    r251036 r252124  
    219219}
    220220
    221 RunLoop::CycleResult RunLoop::cycle(const String&)
     221RunLoop::CycleResult RunLoop::cycle(RunLoopMode)
    222222{
    223223    iterate();

  • trunk/Source/WTF/wtf/glib/RunLoopGLib.cpp

    r251036 r252124  
    124124}
    125125
    126 RunLoop::CycleResult RunLoop::cycle(const String&)
     126RunLoop::CycleResult RunLoop::cycle(RunLoopMode)
    127127{
    128128    g_main_context_iteration(NULL, FALSE);

  • trunk/Source/WTF/wtf/win/RunLoopWin.cpp

    r251036 r252124  
    11/*
    2  * Copyright (C) 2010 Apple Inc. All rights reserved.
     2 * Copyright (C) 2010-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    121121}
    122122
    123 RunLoop::CycleResult RunLoop::cycle(const String&)
     123RunLoop::CycleResult RunLoop::cycle(RunLoopMode)
    124124{
    125125    MSG message;
Note: See TracChangeset for help on using the changeset viewer.