Bug 196588 – The page's focusedFrame / frameSetLargestFrame do not get cleared on process swap or crash

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

The page's focusedFrame / frameSetLargestFrame do not get cleared on process swap or crash.

[Chris Dumez]

<rdar://problem/49365787>

[Chris Dumez]

Created attachment 366693 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 366693 [details]
Patch

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

> Source/WebKit/UIProcess/WebPageProxy.cpp:6779
>      m_mainFrame = nullptr;
> +    m_focusedFrame = nullptr;
> +    m_frameSetLargestFrame = nullptr;

It would be nice to move all the data members which come from a specific web process into a struct, and clear the struct rather than doing it this more fragile way.

[WebKit Commit Bot]

Comment on attachment 366693 [details]
Patch

Clearing flags on attachment: 366693

Committed r243848: <https://trac.webkit.org/changeset/243848>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 366693 [details]
Patch

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

> Tools/TestWebKitAPI/Tests/WebKit/ReloadPageAfterCrash.cpp:141
> +TEST(WebKit, FrameSetLargestFramAfterCrash)

Fram

[Chris Dumez]

(In reply to Darin Adler from comment #6)
> Comment on attachment 366693 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366693&action=review
> 
> > Tools/TestWebKitAPI/Tests/WebKit/ReloadPageAfterCrash.cpp:141
> > +TEST(WebKit, FrameSetLargestFramAfterCrash)
> 
> Fram

Fixed in <https://trac.webkit.org/changeset/243849>, thank you.

[Fujii Hironori]

Windows port can't compile.

> ..\..\Tools\TestWebKitAPI\Tests\WebKit\ReloadPageAfterCrash.cpp(136): error C3861: 'kill': identifier not found

Should I replace 'kill' with WKPageTerminate?

[Ryosuke Niwa]

(In reply to Fujii Hironori from comment #8)
> Windows port can't compile.
> 
> > ..\..\Tools\TestWebKitAPI\Tests\WebKit\ReloadPageAfterCrash.cpp(136): error C3861: 'kill': identifier not found
> 
> Should I replace 'kill' with WKPageTerminate?

Yes!

[Fujii Hironori]

Committed r243858: <https://trac.webkit.org/changeset/243858>

[Chris Dumez]

(In reply to Fujii Hironori from comment #8)
> Windows port can't compile.
> 
> > ..\..\Tools\TestWebKitAPI\Tests\WebKit\ReloadPageAfterCrash.cpp(136): error C3861: 'kill': identifier not found
> 
> Should I replace 'kill' with WKPageTerminate?

I seem to remember the crash handler was not called with wkpageterminate and the test would time out, which is why I used kill.

[Chris Dumez]

(In reply to Chris Dumez from comment #11)
> (In reply to Fujii Hironori from comment #8)
> > Windows port can't compile.
> > 
> > > ..\..\Tools\TestWebKitAPI\Tests\WebKit\ReloadPageAfterCrash.cpp(136): error C3861: 'kill': identifier not found
> > 
> > Should I replace 'kill' with WKPageTerminate?
> 
> I seem to remember the crash handler was not called with wkpageterminate and
> the test would time out, which is why I used kill.

Look at the bots, your follow up change made the tests time out on all platforms... please fix.

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #9)
> (In reply to Fujii Hironori from comment #8)
> > Windows port can't compile.
> > 
> > > ..\..\Tools\TestWebKitAPI\Tests\WebKit\ReloadPageAfterCrash.cpp(136): error C3861: 'kill': identifier not found
> > 
> > Should I replace 'kill' with WKPageTerminate?
> 
> Yes!

No :’(

[Chris Dumez]

Rolled out Fujii's r243858 in <https://trac.webkit.org/changeset/243870> and disabled the new tests on Windows instead for now.