Bug 76425 - [Internals] Should be able to access corresponding Document object.
Summary: [Internals] Should be able to access corresponding Document object.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks: 76424
  Show dependency treegraph
 
Reported: 2012-01-16 21:03 PST by Hajime Morrita
Modified: 2012-01-17 22:55 PST (History)
6 users (show)

See Also:


Attachments
Patch (6.73 KB, patch)
2012-01-17 00:15 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (11.84 KB, patch)
2012-01-17 02:27 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (11.74 KB, patch)
2012-01-17 17:24 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (10.17 KB, patch)
2012-01-17 20:56 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (11.74 KB, patch)
2012-01-17 21:29 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2012-01-16 21:03:59 PST
window.internals.foo(document....) looks too verbose. we can omit the first document parameter.
Comment 1 Hajime Morrita 2012-01-17 00:15:17 PST
Created attachment 122723 [details]
Patch
Comment 2 Hajime Morrita 2012-01-17 00:17:42 PST
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 3 Adam Barth 2012-01-17 01:25:54 PST
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.)
Comment 4 Adam Barth 2012-01-17 01:26:12 PST
Nice idea, by the way.
Comment 5 Hajime Morrita 2012-01-17 02:27:37 PST
Created attachment 122738 [details]
Patch for landing
Comment 6 Hajime Morrita 2012-01-17 02:29:19 PST
Comment on attachment 122738 [details]
Patch for landing

Clearing cq?, waiting for the win bot being green.
Comment 7 Hajime Morrita 2012-01-17 02:32:34 PST
> 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 8 Early Warning System Bot 2012-01-17 02:42:50 PST
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 9 Adam Barth 2012-01-17 02:52:38 PST
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 10 Gyuyoung Kim 2012-01-17 02:53:14 PST
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 11 WebKit Review Bot 2012-01-17 02:56:11 PST
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 12 Collabora GTK+ EWS bot 2012-01-17 06:39:44 PST
Comment on attachment 122738 [details]
Patch for landing

Attachment 122738 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11267180
Comment 13 Hajime Morrita 2012-01-17 16:55:17 PST
(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.
Comment 14 Hajime Morrita 2012-01-17 17:24:09 PST
Created attachment 122846 [details]
Patch for landing
Comment 15 Hajime Morrita 2012-01-17 20:56:40 PST
Created attachment 122868 [details]
Patch
Comment 16 Hajime Morrita 2012-01-17 21:29:29 PST
Created attachment 122870 [details]
Patch for landing
Comment 17 WebKit Review Bot 2012-01-17 22:55:17 PST
Comment on attachment 122870 [details]
Patch for landing

Clearing flags on attachment: 122870

Committed r105245: <http://trac.webkit.org/changeset/105245>
Comment 18 WebKit Review Bot 2012-01-17 22:55:23 PST
All reviewed patches have been landed.  Closing bug.