[Webkit-unassigned] [Bug 109305] [WK2] Page reloading will crash UIProcess after WebProcess was killed
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Feb 11 23:54:20 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=109305
--- Comment #26 from Benjamin Poulain <benjamin at webkit.org> 2013-02-11 23:56:31 PST ---
(From update of attachment 187768)
View in context: https://bugs.webkit.org/attachment.cgi?id=187768&action=review
Some comments:
> Source/WebKit2/ChangeLog:9
> + Re-initialize pointer to WebInspectorProxy object before calling
> + initializeWebPage().
You should also explain why you make the change, not only what the change is.
> Tools/ChangeLog:14
> + * TestWebKitAPI/Tests/WebKit2/CrashReload.cpp: Added.
I'd argue the name CrashReload is a little too generic. It is hard to pinpoint what is tested in there.
> Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:1091
> 2E7765CD16C4D80A00BA2BB1 /* mainIOS.mm in Sources */,
> 2E7765CF16C4D81100BA2BB1 /* mainMac.mm in Sources */,
> + 8A3AF93B16C9ED2700D248C1 /* CrashReload.cpp in Sources */,
The build section should be sorted alphabetically
> Tools/TestWebKitAPI/Tests/WebKit2/CrashReload.cpp:35
> +static bool first = false;
> +static bool second = false;
Those names are too generic.
First, could they be made local instead of being global?
Second, they should be named in a way take make their purpose obvious.
> Tools/TestWebKitAPI/Tests/WebKit2/CrashReload.cpp:39
> + // Ok, this is the first load.
Comments like this are not helping.
> Tools/TestWebKitAPI/Tests/WebKit2/CrashReload.cpp:43
> + if (!first) {
> + first = true;
> + return;
> + }
Should't this also check second is false? If second is true here
> Tools/TestWebKitAPI/Tests/WebKit2/CrashReload.cpp:45
> + // Second time is the charm.
Comments like this are not helping, comments should explain the "Why" of the code.
> Tools/TestWebKitAPI/Tests/WebKit2/CrashReload.cpp:74
> + // Load a blank page and next kills it
Period at the end of the sentence.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list