Summary: | [Internals] Should be able to access corresponding Document object. | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hajime Morrita <morrita> | ||||||||||||
Component: | Tools / Tests | Assignee: | Hajime Morrita <morrita> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | abarth, dglazkov, gustavo.noronha, gustavo, webkit.review.bot, xan.lopez | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 76424 | ||||||||||||||
Attachments: |
|
Description
Hajime Morrita
2012-01-16 21:03:59 PST
Created attachment 122723 [details]
Patch
Hi Dimitri, could you take a look? Using FrameDestructionObserver allows us to access Frame (and Document) without affecting their lifetime. It doesn't touch the reference counts. Comment on attachment 122723 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=122723&action=review > Source/WebCore/page/FrameDestructionObserver.cpp:42 > + Looks like you've got an extra blank line here. > Source/WebCore/page/FrameDestructionObserver.h:37 > + void observe(Frame*); I wonder if we should pick a longer name for this function given that this class will be used as a base class for many other classes. Maybe "observeFrame" ? I leave that up to your judgement. (Please fix the Windows build before landing.) Nice idea, by the way. Created attachment 122738 [details]
Patch for landing
Comment on attachment 122738 [details]
Patch for landing
Clearing cq?, waiting for the win bot being green.
> Nice idea, by the way.
Yeah, such kind of extensibility like FrameDestructionObserver is helpful, not only for testing,
but also for the Modules system you are working on.
I hope "client" objects for modularized API could be
attached to WebCore objects without touching the core part by similar manner.
Comment on attachment 122738 [details] Patch for landing Attachment 122738 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11117146 Comment on attachment 122738 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=122738&action=review > Source/WebCore/page/FrameDestructionObserver.h:37 > + void observeFrame(Frame*); One more thought: should this method be protected? It's probably something that will be called by subclasses but not exposed to others, right? Comment on attachment 122738 [details] Patch for landing Attachment 122738 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11111162 Comment on attachment 122738 [details] Patch for landing Attachment 122738 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11267133 Comment on attachment 122738 [details] Patch for landing Attachment 122738 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11267180 (In reply to comment #9) > > Source/WebCore/page/FrameDestructionObserver.h:37 > > + void observeFrame(Frame*); > > One more thought: should this method be protected? It's probably something that will be called by subclasses but not exposed to others, right? Right. Will do before landing. Created attachment 122846 [details]
Patch for landing
Created attachment 122868 [details]
Patch
Created attachment 122870 [details]
Patch for landing
Comment on attachment 122870 [details] Patch for landing Clearing flags on attachment: 122870 Committed r105245: <http://trac.webkit.org/changeset/105245> All reviewed patches have been landed. Closing bug. |