Bug 107190 - Use WeakPtrs to communicate between the HTMLDocumentParser and the BackgroundHTMLParser
Summary: Use WeakPtrs to communicate between the HTMLDocumentParser and the Background...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on: 107637
Blocks: 106127
  Show dependency tree / graph
 
Reported: 2013-01-17 15:22 PST by Adam Barth
Modified: 2013-02-08 11:32 PST (History)
6 users (show)

See Also:

Attachments
Patch (21.37 KB, patch)
2013-01-17 15:29 PST, Adam Barth 		no flags Details | Formatted Diff | Diff
Patch (15.79 KB, patch)
2013-02-08 11:16 PST, Adam Barth 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 Adam Barth 2013-01-17 15:22:33 PST 
Use WeakPtrs to communicate between the HTMLDocumentParser and the BackgroundHTMLParser
Comment 1 Adam Barth 2013-01-17 15:29:14 PST 
Created attachment 183299 [details]
Patch
Comment 2 Eric Seidel (no email) 2013-01-17 19:48:38 PST 
Comment on attachment 183299 [details]
Patch

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

> Source/WebCore/html/parser/BackgroundHTMLParser.cpp:88
> +void BackgroundHTMLParser::stop()
> +{
> +    delete this;
> +}

Shouldn't it clear itself from its weak ptr factry?
Comment 3 Adam Barth 2013-01-17 21:04:24 PST 
That happens automatically.  The only reason we do it manually in HTMLDocumentParser is because we want to cancel all incoming messages before being destroyed.
Comment 4 Eric Seidel (no email) 2013-01-22 18:43:47 PST 
Comment on attachment 183299 [details]
Patch

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

> Source/WebCore/html/parser/BackgroundHTMLParser.h:45
> +    struct Config {

You mentioned this isn't needed, maybe we should add a FIXME.
Comment 5 Adam Barth 2013-01-22 21:52:32 PST 
I can break this patch into smaller pieces if that would be helpful.  Using WeakPtrs just for the background -> foreground communication would be a much smaller patch.
Comment 6 Eric Seidel (no email) 2013-01-22 22:05:58 PST 
That would help me, yes.
Comment 7 Adam Barth 2013-02-08 11:16:16 PST 
Created attachment 187339 [details]
Patch
Comment 8 Eric Seidel (no email) 2013-02-08 11:23:51 PST 
Comment on attachment 187339 [details]
Patch

This is so much better.  Thank you.
Comment 9 Adam Barth 2013-02-08 11:32:21 PST 
Comment on attachment 187339 [details]
Patch

Clearing flags on attachment: 187339

Committed r142305: <http://trac.webkit.org/changeset/142305>
Comment 10 Adam Barth 2013-02-08 11:32:25 PST 
All reviewed patches have been landed.  Closing bug.