Bug 85963 - Node::InDetachFlag could be removed.
Summary: Node::InDetachFlag could be removed.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks: 77860
  Show dependency tree / graph
 
Reported: 2012-05-09 01:26 PDT by Hajime Morrita
Modified: 2012-05-10 16:48 PDT (History)
2 users (show)

See Also:

Attachments
Patch (5.05 KB, patch)
2012-05-09 01:58 PDT, Hajime Morrita 		no flags Details | Formatted Diff | Diff
Patch (5.50 KB, patch)
2012-05-09 02:25 PDT, Hajime Morrita 		no flags Details | Formatted Diff | Diff
Patch for landing (5.58 KB, patch)
2012-05-10 04:59 PDT, Hajime Morrita 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.