Bug 87086 - PAGE_POPUP: window.setValueAndClosePopup should be moved to a per-context property of DOMWindow.
: PAGE_POPUP: window.setValueAndClosePopup should be moved to a per-context pro...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To: Kent Tamura
:
Depends on: 86555 87185
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-21 23:29 PDT by Kent Tamura
Modified: 2012-05-24 23:51 PDT (History)
6 users (show)

See Also:


Attachments
WIP (21.95 KB, patch)
2012-05-22 02:52 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
WIP 2 (22.48 KB, patch)
2012-05-22 02:55 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch (24.87 KB, patch)
2012-05-22 23:57 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch for landing (27.20 KB, patch)
2012-05-24 22:43 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-05-21 23:29:54 PDT
[V8] PAGE_POPUP: window.setValueAndClosePopup should be a per-context function
Comment 1 Kent Tamura 2012-05-22 02:52:11 PDT
Created attachment 143245 [details]
WIP
Comment 2 Kent Tamura 2012-05-22 02:55:31 PDT
Created attachment 143246 [details]
WIP 2
Comment 3 Hajime Morrita 2012-05-22 21:01:50 PDT
Comment on attachment 143246 [details]
WIP 2

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

Maybe we could define window.popupController attribute instead of function.
Then popup window can turn to be a supplemental module.

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:368
> +        if ($attrExt->{"V8EnablePerContext"}) {

EnableAtRuntime and EnablePerContext should be mutually exclusive. So else if would be preferred.
Comment 4 Kent Tamura 2012-05-22 22:58:05 PDT
> Maybe we could define window.popupController attribute instead of function.
> Then popup window can turn to be a supplemental module.

It make sense.  It's extensible and I don't need to touch a Perl code though we need to add one more C++ class and IDL.
Comment 5 Kent Tamura 2012-05-22 23:57:33 PDT
Created attachment 143479 [details]
Patch
Comment 6 Adam Barth 2012-05-23 10:50:58 PDT
Comment on attachment 143479 [details]
Patch

Should we add a test that pagePopupController doesn't appear on the default Window object by mistake?
Comment 7 Hajime Morrita 2012-05-23 17:13:46 PDT
(In reply to comment #6)
> (From update of attachment 143479 [details])
> Should we add a test that pagePopupController doesn't appear on the default Window object by mistake?
That sounds nice. Just having a test which checks pagePopController 
will be sufficient.
If we have a way to enable it from testing API, it would be greater.
But that is another story.
Comment 8 Kent Tamura 2012-05-24 22:43:06 PDT
Created attachment 143980 [details]
Patch for landing

Add a simple test
Comment 9 WebKit Review Bot 2012-05-24 23:50:55 PDT
Comment on attachment 143980 [details]
Patch for landing

Clearing flags on attachment: 143980

Committed r118482: <http://trac.webkit.org/changeset/118482>
Comment 10 WebKit Review Bot 2012-05-24 23:51:02 PDT
All reviewed patches have been landed.  Closing bug.