Skip to content

Commit

Permalink
ProcessThrottler should use smart pointers instead of raw pointers
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=259500

Reviewed by Chris Dumez.

Deploy smart pointers around ProcessThrottler.

* Source/WebKit/UIProcess/ProcessThrottler.cpp:
(WebKit::ProcessThrottler::addActivity):
(WebKit::ProcessThrottler::removeActivity):
(WebKit::ProcessThrottler::invalidateAllActivities):
(WebKit::ProcessThrottler::expectedThrottleState):
(WebKit::ProcessThrottler::setThrottleState):
(WebKit::ProcessThrottler::setAllowsActivities):
(WebKit::logActivityNames):
* Source/WebKit/UIProcess/ProcessThrottler.h:
(WebKit::ProcessThrottlerTimedActivity::activity const):
(WebKit::ProcessThrottler::shouldBeRunnable const):

Canonical link: https://commits.webkit.org/266496@main
  • Loading branch information
@rniwa
rniwa committed Aug 1, 2023
1 parent c01b5bf commit 0d88eeb
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 38 deletions.
40 changes: 20 additions & 20 deletions Source/WebKit/UIProcess/ProcessThrottler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ bool ProcessThrottler::addActivity(Activity& activity)
}

if (activity.isForeground())
m_foregroundActivities.add(&activity);
m_foregroundActivities.add(activity);
else
m_backgroundActivities.add(&activity);
m_backgroundActivities.add(activity);
updateThrottleStateIfNeeded();
return true;
}
Expand All @@ -87,16 +87,16 @@ void ProcessThrottler::removeActivity(Activity& activity)
{
ASSERT(isMainRunLoop());
if (!m_allowsActivities) {
ASSERT(m_foregroundActivities.isEmpty());
ASSERT(m_backgroundActivities.isEmpty());
ASSERT(m_foregroundActivities.isEmptyIgnoringNullReferences());
ASSERT(m_backgroundActivities.isEmptyIgnoringNullReferences());
return;
}

bool wasRemoved;
if (activity.isForeground())
wasRemoved = m_foregroundActivities.remove(&activity);
wasRemoved = m_foregroundActivities.remove(activity);
else
wasRemoved = m_backgroundActivities.remove(&activity);
wasRemoved = m_backgroundActivities.remove(activity);
ASSERT(wasRemoved);
if (!wasRemoved)
return;
Expand All @@ -107,11 +107,11 @@ void ProcessThrottler::removeActivity(Activity& activity)
void ProcessThrottler::invalidateAllActivities()
{
ASSERT(isMainRunLoop());
PROCESSTHROTTLER_RELEASE_LOG("invalidateAllActivities: BEGIN (foregroundActivityCount: %u, backgroundActivityCount: %u)", m_foregroundActivities.size(), m_backgroundActivities.size());
while (!m_foregroundActivities.isEmpty())
(*m_foregroundActivities.begin())->invalidate();
while (!m_backgroundActivities.isEmpty())
(*m_backgroundActivities.begin())->invalidate();
PROCESSTHROTTLER_RELEASE_LOG("invalidateAllActivities: BEGIN (foregroundActivityCount: %u, backgroundActivityCount: %u)", m_foregroundActivities.computeSize(), m_backgroundActivities.computeSize());
while (!m_foregroundActivities.isEmptyIgnoringNullReferences())
m_foregroundActivities.begin()->invalidate();
while (!m_backgroundActivities.isEmptyIgnoringNullReferences())
m_backgroundActivities.begin()->invalidate();
PROCESSTHROTTLER_RELEASE_LOG("invalidateAllActivities: END");
}

Expand All @@ -126,9 +126,9 @@ void ProcessThrottler::invalidateAllActivitiesAndDropAssertion()

ProcessThrottleState ProcessThrottler::expectedThrottleState()
{
if (!m_foregroundActivities.isEmpty())
if (!m_foregroundActivities.isEmptyIgnoringNullReferences())
return ProcessThrottleState::Foreground;
if (!m_backgroundActivities.isEmpty())
if (!m_backgroundActivities.isEmptyIgnoringNullReferences())
return ProcessThrottleState::Background;
return ProcessThrottleState::Suspended;
}
Expand Down Expand Up @@ -184,7 +184,7 @@ void ProcessThrottler::setThrottleState(ProcessThrottleState newState)
if (m_assertion && m_assertion->isValid() && m_assertion->type() == newType)
return;

PROCESSTHROTTLER_RELEASE_LOG("setThrottleState: Updating process assertion type to %u (foregroundActivities=%u, backgroundActivities=%u)", WTF::enumToUnderlyingType(newType), m_foregroundActivities.size(), m_backgroundActivities.size());
PROCESSTHROTTLER_RELEASE_LOG("setThrottleState: Updating process assertion type to %u (foregroundActivities=%u, backgroundActivities=%u)", WTF::enumToUnderlyingType(newType), m_foregroundActivities.computeSize(), m_backgroundActivities.computeSize());

// Keep the previous assertion active until the new assertion is taken asynchronously.
auto previousAssertion = std::exchange(m_assertion, nullptr);
Expand Down Expand Up @@ -359,8 +359,8 @@ void ProcessThrottler::setAllowsActivities(bool allow)
invalidateAllActivities();
}

ASSERT(m_foregroundActivities.isEmpty());
ASSERT(m_backgroundActivities.isEmpty());
ASSERT(m_foregroundActivities.isEmptyIgnoringNullReferences());
ASSERT(m_backgroundActivities.isEmptyIgnoringNullReferences());
m_allowsActivities = allow;
}

Expand Down Expand Up @@ -473,18 +473,18 @@ void ProcessThrottlerTimedActivity::updateTimer()
template <typename T>
static void logActivityNames(WTF::TextStream& ts, ASCIILiteral description, const T& activities, bool& didLog)
{
if (!activities.size())
if (activities.isEmptyIgnoringNullReferences())
return;

ts << (didLog ? ", "_s : ""_s) << description << ": "_s;
didLog = true;

bool isFirstItem = true;
for (const auto* activity : activities) {
if (activity && !activity->isQuietActivity()) {
for (const auto& activity : activities) {
if (!activity.isQuietActivity()) {
if (!isFirstItem)
ts << ", "_s;
ts << activity->name();
ts << activity.name();
isFirstItem = false;
}
}
Expand Down
36 changes: 18 additions & 18 deletions Source/WebKit/UIProcess/ProcessThrottler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ enum class IsSuspensionImminent : bool { No, Yes };
enum class ProcessThrottleState : uint8_t { Suspended, Background, Foreground };
enum class ProcessThrottlerActivityType : bool { Background, Foreground };

class ProcessThrottlerActivity {
class ProcessThrottlerActivity : public CanMakeWeakPtr<ProcessThrottlerActivity> {
WTF_MAKE_FAST_ALLOCATED;
WTF_MAKE_NONCOPYABLE(ProcessThrottlerActivity);
public:
Expand All @@ -80,7 +80,7 @@ class ProcessThrottlerActivity {

void invalidate();

ProcessThrottler* m_throttler { nullptr };
WeakPtr<ProcessThrottler> m_throttler;
ASCIILiteral m_name;
ProcessThrottlerActivityType m_type;
};
Expand All @@ -90,18 +90,18 @@ class ProcessThrottlerTimedActivity {
WTF_MAKE_NONCOPYABLE(ProcessThrottlerTimedActivity);
using Activity = ProcessThrottlerActivity;
using ActivityVariant = std::variant<std::nullptr_t, UniqueRef<Activity>>;
public:
explicit ProcessThrottlerTimedActivity(Seconds timeout, ActivityVariant&& = nullptr);
ProcessThrottlerTimedActivity& operator=(ActivityVariant&&);
const ActivityVariant& activity() const { return m_activity; }

private:
void activityTimedOut();
void updateTimer();

RunLoop::Timer m_timer;
Seconds m_timeout;
ActivityVariant m_activity;
public:
explicit ProcessThrottlerTimedActivity(Seconds timeout, ActivityVariant&& = nullptr);
ProcessThrottlerTimedActivity& operator=(ActivityVariant&&);
const ActivityVariant& activity() const { return m_activity; }

private:
void activityTimedOut();
void updateTimer();

RunLoop::Timer m_timer;
Seconds m_timeout;
ActivityVariant m_activity;
};

class ProcessThrottler : public CanMakeWeakPtr<ProcessThrottler> {
Expand Down Expand Up @@ -130,7 +130,7 @@ class ProcessThrottler : public CanMakeWeakPtr<ProcessThrottler> {

void didConnectToProcess(ProcessID);
void didDisconnectFromProcess();
bool shouldBeRunnable() const { return m_foregroundActivities.size() || m_backgroundActivities.size(); }
bool shouldBeRunnable() const { return !m_foregroundActivities.isEmptyIgnoringNullReferences() || !m_backgroundActivities.isEmptyIgnoringNullReferences(); }
void setAllowsActivities(bool);
void setShouldDropNearSuspendedAssertionAfterDelay(bool);
void setShouldTakeNearSuspendedAssertion(bool);
Expand Down Expand Up @@ -175,8 +175,8 @@ class ProcessThrottler : public CanMakeWeakPtr<ProcessThrottler> {
RunLoop::Timer m_prepareToSuspendTimeoutTimer;
RunLoop::Timer m_dropNearSuspendedAssertionTimer;
RunLoop::Timer m_prepareToDropLastAssertionTimeoutTimer;
HashSet<Activity*> m_foregroundActivities;
HashSet<Activity*> m_backgroundActivities;
WeakHashSet<Activity> m_foregroundActivities;
WeakHashSet<Activity> m_backgroundActivities;
std::optional<uint64_t> m_pendingRequestToSuspendID;
ProcessThrottleState m_state { ProcessThrottleState::Suspended };
PageAllowedToRunInTheBackgroundCounter m_pageAllowedToRunInTheBackgroundCounter;
Expand All @@ -186,7 +186,7 @@ class ProcessThrottler : public CanMakeWeakPtr<ProcessThrottler> {
bool m_allowsActivities { true };
};

#define PROCESSTHROTTLER_ACTIVITY_RELEASE_LOG(msg, ...) RELEASE_LOG(ProcessSuspension, "%p - [PID=%d, throttler=%p] ProcessThrottler::Activity::" msg, this, m_throttler->m_processID, m_throttler, ##__VA_ARGS__)
#define PROCESSTHROTTLER_ACTIVITY_RELEASE_LOG(msg, ...) RELEASE_LOG(ProcessSuspension, "%p - [PID=%d, throttler=%p] ProcessThrottler::Activity::" msg, this, m_throttler->m_processID, m_throttler.get(), ##__VA_ARGS__)

inline ProcessThrottlerActivity::ProcessThrottlerActivity(ProcessThrottler& throttler, ASCIILiteral name, ProcessThrottlerActivityType type)
: m_throttler(&throttler)
Expand Down

0 comments on commit 0d88eeb

Please sign in to comment.