RESOLVED FIXED Bug 85963
Node::InDetachFlag could be removed. 
https://bugs.webkit.org/show_bug.cgi?id=85963
Summary Node::InDetachFlag could be removed.
Hajime Morrita
Reported 2012-05-09 01:26:19 PDT
It looks there are only one node which could have the flag at the same time. That means the flag can be replaced with a global variable.
Attachments
Patch (5.05 KB, patch)
2012-05-09 01:58 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch (5.50 KB, patch)
2012-05-09 02:25 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch for landing (5.58 KB, patch)
2012-05-10 04:59 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2012-05-09 01:58:06 PDT
Created attachment 140888 [details] Patch
Antti Koivisto
Comment 2 2012-05-09 02:14:23 PDT
Comment on attachment 140888 [details] Patch Looks like we could easily remove isDetach() as it has just one client that probably never gets hit.
Hajime Morrita
Comment 3 2012-05-09 02:25:56 PDT
Created attachment 140891 [details] Patch
Antti Koivisto
Comment 4 2012-05-09 02:29:01 PDT
Comment on attachment 140891 [details] Patch r=me
Hajime Morrita
Comment 5 2012-05-10 04:59:17 PDT
Created attachment 141147 [details] Patch for landing
WebKit Review Bot
Comment 6 2012-05-10 07:42:43 PDT
Comment on attachment 141147 [details] Patch for landing Clearing flags on attachment: 141147 Committed r116644: <http://trac.webkit.org/changeset/116644>
WebKit Review Bot
Comment 7 2012-05-10 07:42:47 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 8 2012-05-10 11:15:49 PDT
What proves inDetach can never be true? Can’t some of that code in detach result in calls out to arbitrary JavaScript? I am not sure this is correct.
Hajime Morrita
Comment 9 2012-05-10 16:48:20 PDT
Hi Darin, thanks for the feedback! (In reply to comment #8) > What proves inDetach can never be true? Can’t some of that code in detach result in calls out to arbitrary JavaScript? I am not sure this is correct. The point is that this is not arbitrary detach() but Node::detach(). I checked the code and there is no such code path. But I agree that I might be too optimistic. I'll post a follow up at Bug 86157.
Note You need to log in before you can comment on or make changes to this bug.