Bug 85963

Summary: Node::InDetachFlag could be removed.
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: DOMAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 77860    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Hajime Morrita 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.
Comment 1 Hajime Morrita 2012-05-09 01:58:06 PDT 
Created attachment 140888 [details]
Patch
Comment 2 Antti Koivisto 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.
Comment 3 Hajime Morrita 2012-05-09 02:25:56 PDT 
Created attachment 140891 [details]
Patch
Comment 4 Antti Koivisto 2012-05-09 02:29:01 PDT 
Comment on attachment 140891 [details]
Patch

r=me
Comment 5 Hajime Morrita 2012-05-10 04:59:17 PDT 
Created attachment 141147 [details]
Patch for landing
Comment 6 WebKit Review Bot 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>
Comment 7 WebKit Review Bot 2012-05-10 07:42:47 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Darin Adler 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.
Comment 9 Hajime Morrita 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.