Add a new switch: --allow-scripts-to-close-windows....

Add a new switch: --allow-scripts-to-close-windows.

With this switch, window.close() will always be enabled.

Currently the switch is only available in test_shell.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=2333

[Feng Qian, 2008-09-17 02:45:27]

See my comments. My concern is the change to Frame.h & Frame.cpp. Can we avoid
that? Otherwise, LGTM

http://codereview.chromium.org/2915/diff/14/217
File webkit/pending/DOMWindow.cpp (right):

http://codereview.chromium.org/2915/diff/14/217#newcode348
Line 348: if (m_frame->loader()->openedByDOM() ||
m_frame->loader()->getHistoryLength() <= 1 ||
m_frame->allowScriptsToCloseWindows())
You may want to break this line into three lines.

http://codereview.chromium.org/2915/diff/14/214
File webkit/pending/Frame.cpp (right):

http://codereview.chromium.org/2915/diff/14/214#newcode1197
Line 1197: return s ? s->allowScriptsToCloseWindows() : false;
Can we move the logic into the caller so we can avoid making more changes in
Frame.*? My understanding is that we try to folk less files.

http://codereview.chromium.org/2915/diff/14/224
File webkit/tools/test_shell/test_shell_switches.cc (right):

http://codereview.chromium.org/2915/diff/14/224#newcode58
Line 58: const wchar_t kAllowScriptsToCloseWindows[] =
L"allow-scripts-to-close-windows";
allow-scripts-to-close-window sounds too long, can you make a short version?

[Patrick Johnson, 2008-09-17 03:28:55]

http://codereview.chromium.org/2915/diff/14/217
File webkit/pending/DOMWindow.cpp (right):

http://codereview.chromium.org/2915/diff/14/217#newcode348
Line 348: if (m_frame->loader()->openedByDOM() ||
m_frame->loader()->getHistoryLength() <= 1 ||
m_frame->allowScriptsToCloseWindows())
On 2008/09/17 02:45:27, Feng Qian wrote:
> You may want to break this line into three lines.

Done.  I tried to follow WebKit style.

http://codereview.chromium.org/2915/diff/14/214
File webkit/pending/Frame.cpp (right):

http://codereview.chromium.org/2915/diff/14/214#newcode1197
Line 1197: return s ? s->allowScriptsToCloseWindows() : false;
On 2008/09/17 02:45:27, Feng Qian wrote:
> Can we move the logic into the caller so we can avoid making more changes in
> Frame.*? My understanding is that we try to folk less files.

Okay, I moved the logic out of Frame.cpp.  No more changes in Frame.*.

http://codereview.chromium.org/2915/diff/14/224
File webkit/tools/test_shell/test_shell_switches.cc (right):

http://codereview.chromium.org/2915/diff/14/224#newcode58
Line 58: const wchar_t kAllowScriptsToCloseWindows[] =
L"allow-scripts-to-close-windows";
On 2008/09/17 02:45:27, Feng Qian wrote:
> allow-scripts-to-close-window sounds too long, can you make a short version?

I picked that name to match dom.allow_scripts_to_close_windows in Firefox.  It
is long and IMO it's a good name but not perfect.

Something shorter could be: allow-script-close, but this might be confusing.