Changeset 265162 in webkit


Ignore:
Timestamp:
Jul 31, 2020 3:23:06 PM (4 years ago)
Author:
Chris Dumez
Message:

Drop ProcessAssertion::Client and replace with a simple invalidation handler
​https://bugs.webkit.org/show_bug.cgi?id=214976

Reviewed by Brady Eidson.

ProcessAssertion had a Client class with 2 functions:
(1) one to indicate that the assertion was invalidated
(2) one to indicate that the UI assertion is about to expire

While ProcessAssertion is used in several places, only the ProcessThrottler
ever sets itself as a client. The reason is that other call sites use
assertion types that do not expire. Also, (2) only makes sense for
ProcessAndUIAssertion, not for ProcessAssertion. Only the ProcessThrottler
is using a ProcessAndUIAssertion.

I think a better design is to have a simple invalidation handler on
the ProcessAssertion, that the call site can set if they are interested
in invalidation.

Similarly, I added a UIAssertion expiration handler on ProcessAndUIAssertion
so that the ProcessThrottler can know if the UIAssertion is about to expire.

This new design also matches more closely the system process assertion API
that ProcessAssertion / ProcessAndUIAssertion are wrapping.

This patch also fixes a bug found by Youenn Fablet where the ProcessThrottler
would only set its invalidation handler in didConnectToProcess(), instead of
doing it every time a new ProcessAssertion is created in setAssertionType().
The assertion type can change (e.g. from foreground to background) after the
process has launched.

  • UIProcess/ProcessAssertion.h:

(WebKit::ProcessAssertion::setInvalidationHandler):
(WebKit::ProcessAssertion::Client::~Client): Deleted.
(WebKit::ProcessAssertion::setClient): Deleted.
(WebKit::ProcessAssertion::client): Deleted.

  • UIProcess/ProcessThrottler.cpp:

(WebKit::ProcessThrottler::setAssertionType):
(WebKit::ProcessThrottler::didConnectToProcess):

  • UIProcess/ProcessThrottler.h:
  • UIProcess/ios/ProcessAssertionIOS.mm:

(WebKit::ProcessAssertion::processAssertionWasInvalidated):
(WebKit::ProcessAndUIAssertion::uiAssertionWillExpireImminently):

Location:
trunk/Source/WebKit
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r265158 r265162  
     12020-07-31  Chris Dumez  <cdumez@apple.com>
     2
     3        Drop ProcessAssertion::Client and replace with a simple invalidation handler
     4        https://bugs.webkit.org/show_bug.cgi?id=214976
     5
     6        Reviewed by Brady Eidson.
     7
     8        ProcessAssertion had a Client class with 2 functions:
     9        (1) one to indicate that the assertion was invalidated
     10        (2) one to indicate that the UI assertion is about to expire
     11
     12        While ProcessAssertion is used in several places, only the ProcessThrottler
     13        ever sets itself as a client. The reason is that other call sites use
     14        assertion types that do not expire. Also, (2) only makes sense for
     15        ProcessAndUIAssertion, not for ProcessAssertion. Only the ProcessThrottler
     16        is using a ProcessAndUIAssertion.
     17
     18        I think a better design is to have a simple invalidation handler on
     19        the ProcessAssertion, that the call site can set if they are interested
     20        in invalidation.
     21
     22        Similarly, I added a UIAssertion expiration handler on ProcessAndUIAssertion
     23        so that the ProcessThrottler can know if the UIAssertion is about to expire.
     24
     25        This new design also matches more closely the system process assertion API
     26        that ProcessAssertion / ProcessAndUIAssertion are wrapping.
     27
     28        This patch also fixes a bug found by Youenn Fablet where the ProcessThrottler
     29        would only set its invalidation handler in didConnectToProcess(), instead of
     30        doing it every time a new ProcessAssertion is created in setAssertionType().
     31        The assertion type can change (e.g. from foreground to background) after the
     32        process has launched.
     33
     34        * UIProcess/ProcessAssertion.h:
     35        (WebKit::ProcessAssertion::setInvalidationHandler):
     36        (WebKit::ProcessAssertion::Client::~Client): Deleted.
     37        (WebKit::ProcessAssertion::setClient): Deleted.
     38        (WebKit::ProcessAssertion::client): Deleted.
     39        * UIProcess/ProcessThrottler.cpp:
     40        (WebKit::ProcessThrottler::setAssertionType):
     41        (WebKit::ProcessThrottler::didConnectToProcess):
     42        * UIProcess/ProcessThrottler.h:
     43        * UIProcess/ios/ProcessAssertionIOS.mm:
     44        (WebKit::ProcessAssertion::processAssertionWasInvalidated):
     45        (WebKit::ProcessAndUIAssertion::uiAssertionWillExpireImminently):
     46
    1472020-07-31  Jiewen Tan  <jiewen_tan@apple.com>
    248

  • trunk/Source/WebKit/UIProcess/ProcessAssertion.cpp

    r261034 r265162  
    4646}
    4747
     48ProcessAndUIAssertion::ProcessAndUIAssertion(ProcessID pid, const String& reason, ProcessAssertionType assertionType)
     49    : ProcessAssertion(pid, reason, assertionType)
     50{
     51}
     52
     53ProcessAndUIAssertion::~ProcessAndUIAssertion() = default;
     54
    4855} // namespace WebKit
    4956

  • trunk/Source/WebKit/UIProcess/ProcessAssertion.h

    r262566 r265162  
    5959    WTF_MAKE_FAST_ALLOCATED;
    6060public:
    61     class Client {
    62     public:
    63         virtual ~Client() { }
    64 
    65         virtual void uiAssertionWillExpireImminently() = 0;
    66         virtual void assertionWasInvalidated() = 0;
    67     };
    68 
    6961    ProcessAssertion(ProcessID, const String& reason, ProcessAssertionType);
    7062    virtual ~ProcessAssertion();
    7163
    72     void setClient(Client& client) { m_client = &client; }
    73     Client* client() { return m_client; }
     64    void setInvalidationHandler(Function<void()>&& handler) { m_invalidationHandler = WTFMove(handler); }
    7465
    7566    ProcessAssertionType type() const { return m_assertionType; }
    … …  
    9384#endif
    9485#endif
    95     Client* m_client { nullptr };
     86    Function<void()> m_invalidationHandler;
    9687};
    97 
    98 #if PLATFORM(IOS_FAMILY)
    9988
    10089class ProcessAndUIAssertion final : public ProcessAssertion {
    … …  
    10594    void uiAssertionWillExpireImminently();
    10695
     96    void setUIAssertionExpirationHandler(Function<void()>&& handler) { m_uiAssertionExpirationHandler = WTFMove(handler); }
     97
    10798private:
     99#if PLATFORM(IOS_FAMILY)
    108100    void processAssertionWasInvalidated() final;
     101#endif
    109102    void updateRunInBackgroundCount();
    110103
     104    Function<void()> m_uiAssertionExpirationHandler;
    111105    bool m_isHoldingBackgroundTask { false };
    112106};
    113 
    114 #else
    115 
    116 using ProcessAndUIAssertion = ProcessAssertion;
    117 
    118 #endif // PLATFORM(IOS_FAMILY)
    119107   
    120108} // namespace WebKit

  • trunk/Source/WebKit/UIProcess/ProcessThrottler.cpp

    r262566 r265162  
    128128
    129129    PROCESSTHROTTLER_RELEASE_LOG("setAssertionType: Updating process assertion type to %u (foregroundActivities: %u, backgroundActivities: %u)", newType, m_foregroundActivities.size(), m_backgroundActivities.size());
    130     if (m_shouldTakeUIBackgroundAssertion)
    131         m_assertion = makeUnique<ProcessAndUIAssertion>(m_processIdentifier, assertionName(newType), newType);
    132     else
     130    if (m_shouldTakeUIBackgroundAssertion) {
     131        auto assertion = makeUnique<ProcessAndUIAssertion>(m_processIdentifier, assertionName(newType), newType);
     132        assertion->setUIAssertionExpirationHandler([this] {
     133            uiAssertionWillExpireImminently();
     134        });
     135        m_assertion = WTFMove(assertion);
     136    } else
    133137        m_assertion = makeUnique<ProcessAssertion>(m_processIdentifier, assertionName(newType), newType);
     138
     139    m_assertion->setInvalidationHandler([this] {
     140        assertionWasInvalidated();
     141    });
    134142    m_process.didSetAssertionType(newType);
    135143}
    … …  
    171179    setAssertionType(expectedAssertionType());
    172180    RELEASE_ASSERT(m_assertion);
    173     m_assertion->setClient(*this);
    174181}
    175182   

  • trunk/Source/WebKit/UIProcess/ProcessThrottler.h

    r261288 r265162  
    5252class ProcessThrottlerClient;
    5353
    54 class ProcessThrottler : public CanMakeWeakPtr<ProcessThrottler>, private ProcessAssertion::Client {
     54class ProcessThrottler : public CanMakeWeakPtr<ProcessThrottler> {
    5555public:
    5656    ProcessThrottler(ProcessThrottlerClient&, bool shouldTakeUIBackgroundAssertion);
    … …  
    143143    String assertionName(ProcessAssertionType) const;
    144144
    145     // ProcessAssertionClient
    146     void uiAssertionWillExpireImminently() final;
    147     void assertionWasInvalidated() final;
     145    void uiAssertionWillExpireImminently();
     146    void assertionWasInvalidated();
    148147
    149148    void clearPendingRequestToSuspend();

  • trunk/Source/WebKit/UIProcess/ios/ProcessAssertionIOS.mm

    r263306 r265162  
    453453    RELEASE_LOG(ProcessSuspension, "%p - ProcessAssertion::processAssertionWasInvalidated() PID: %d", this, m_pid);
    454454
    455     if (auto* client = this->client())
    456         client->assertionWasInvalidated();
     455    if (m_invalidationHandler)
     456        m_invalidationHandler();
    457457}
    458458
    … …  
    498498void ProcessAndUIAssertion::uiAssertionWillExpireImminently()
    499499{
    500     if (auto* client = this->client())
    501         client->uiAssertionWillExpireImminently();
     500    if (m_uiAssertionExpirationHandler)
     501        m_uiAssertionExpirationHandler();
    502502}
    503503
Note: See TracChangeset for help on using the changeset viewer.