Bug 195024 – Network Process is put to suspended when holding locked IndexedDB files

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sihui Liu]

Network process should take a background task assertion so that it won't get suspended when it's holding locked files.

[Sihui Liu]

<rdar://problem/45194169>

[Sihui Liu]

Created attachment 362943 [details]
Patch

[Geoffrey Garen]

Comment on attachment 362943 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362943&action=review

> Source/WebCore/ChangeLog:9
> +        We found network process was put to suspension when IDBDatabase was being closed in the background database

was put to suspension when => was suspended while

> Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:47
> +    WTFReportBacktrace();

Please revert.

> Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:53
> +    WTFReportBacktrace();

Please revert.

> Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:69
> +    , m_notifyProcessFunction(WTFMove(notifyProcessFunction))

I'm having a hard time understanding what this function pointer does. I think it needs a better name. Can you describe it in words, and maybe we can come up with a name?

[Sihui Liu]

(In reply to Geoffrey Garen from comment #3)
> Comment on attachment 362943 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=362943&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        We found network process was put to suspension when IDBDatabase was being closed in the background database
> 
> was put to suspension when => was suspended while
> 
Okay.

> > Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:47
> > +    WTFReportBacktrace();
> 
> Please revert.
> 
> > Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:53
> > +    WTFReportBacktrace();
> 
> Please revert.
> 
Oops, sure.

> > Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:69
> > +    , m_notifyProcessFunction(WTFMove(notifyProcessFunction))
> 
> I'm having a hard time understanding what this function pointer does. I
> think it needs a better name. Can you describe it in words, and maybe we can
> come up with a name?

This function pointer let IDBServer send a message via connection of NetworkProcess, since IDBServer doesn't have a NetworkProcess member. That's why I called it notify(Network)ProcessFunction...

[Geoffrey Garen]

Comment on attachment 362943 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362943&action=review

> Source/WebCore/Modules/indexeddb/server/IDBServer.h:144
> +    WTF::Function<void(bool)> m_notifyProcessFunction;

Let's call this m_isClosingDatabaseCallback.

> Source/WebCore/Modules/indexeddb/server/IDBServer.h:145
> +    PAL::HysteresisActivity m_hysteresis;

Let's call this m_isClosingDatabaseHysteresis.

Would that make sense?

[Sihui Liu]

(In reply to Geoffrey Garen from comment #5)
> Comment on attachment 362943 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=362943&action=review
> 
> > Source/WebCore/Modules/indexeddb/server/IDBServer.h:144
> > +    WTF::Function<void(bool)> m_notifyProcessFunction;
> 
> Let's call this m_isClosingDatabaseCallback.
> 
Okay.

> > Source/WebCore/Modules/indexeddb/server/IDBServer.h:145
> > +    PAL::HysteresisActivity m_hysteresis;
> 
> Let's call this m_isClosingDatabaseHysteresis.
> 
Okay.

> Would that make sense?
>
Looks good.

[Sihui Liu]

Created attachment 363018 [details]
Patch

[Geoffrey Garen]

Comment on attachment 363018 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 363018 [details]
Patch

Clearing flags on attachment: 363018

Committed r242136: <https://trac.webkit.org/changeset/242136>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.