[webkit-reviews] review denied: [Bug 95681] Move PagePopupClient to WebKit layer : [Attachment 162011] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Keishi Hattori
<keishi at webkit.org>'s request for review:
Bug 95681: Move PagePopupClient to WebKit layer
https://bugs.webkit.org/show_bug.cgi?id=95681

Attachment 162011: Patch
https://bugs.webkit.org/attachment.cgi?id=162011&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=162011&action=review


> Source/WebKit/chromium/ChangeLog:59
> +2012-09-04  Keishi Hattori  <keishi at webkit.org>
> +
> +	   Move PagePopupClient to WebKit layer
> +	   https://bugs.webkit.org/show_bug.cgi?id=95681
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   * WebKit.gyp:
> +	   * src/CalendarPickerPopup.cpp: Added.
> +	   (WebKit):
> +	   (WebKit::CalendarPickerPopup::CalendarPickerPopup):
> +	   (WebKit::CalendarPickerPopup::~CalendarPickerPopup):
> +	   (WebKit::CalendarPickerPopup::endChooser):
> +	   (WebKit::CalendarPickerPopup::contentSize):
> +	   (WebKit::CalendarPickerPopup::writeDocument):
> +	   (WebKit::CalendarPickerPopup::setValueAndClosePopup):
> +	   (WebKit::CalendarPickerPopup::didClosePopup):
> +	   * src/CalendarPickerPopup.h: Copied from
Source/WebCore/html/shadow/CalendarPickerElement.h.
> +	   (WebCore):
> +	   (WebKit):
> +	   (CalendarPickerPopup):
> +	   * src/ChromeClientImpl.cpp:
> +	   (WebKit):
> +	   (WebKit::ChromeClientImpl::openCalendarPickerPopup):
> +	   * src/ChromeClientImpl.h:
> +	   (WebCore):
> +	   (ChromeClientImpl):
> +
> +2012-09-04  Keishi Hattori  <keishi at webkit.org>
> +
> +	   Move PagePopupClient to WebKit layer
> +	   https://bugs.webkit.org/show_bug.cgi?id=95681
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Additional information of the change such as approach, rationale.
Please add per-function descriptions below (OOPS!).
> +
> +	   * WebKit.gyp:
> +	   * src/CalendarPickerPopup.cpp: Added.
> +	   (WebKit):
> +	   (WebKit::CalendarPickerPopup::CalendarPickerPopup):
> +	   (WebKit::CalendarPickerPopup::~CalendarPickerPopup):
> +	   (WebKit::CalendarPickerPopup::endChooser):
> +	   (WebKit::CalendarPickerPopup::contentSize):
> +	   (WebKit::CalendarPickerPopup::writeDocument):
> +	   (WebKit::CalendarPickerPopup::setValueAndClosePopup):
> +	   (WebKit::CalendarPickerPopup::didClosePopup):
> +	   * src/CalendarPickerPopup.h: Copied from
Source/WebCore/html/shadow/CalendarPickerElement.h.
> +	   (WebCore):
> +	   (WebKit):
> +	   (CalendarPickerPopup):
> +	   * src/ChromeClientImpl.cpp:
> +	   (WebKit):
> +	   (WebKit::ChromeClientImpl::openCalendarPickerPopup):
> +	   * src/ChromeClientImpl.h:
> +	   (WebCore):
> +	   (ChromeClientImpl):
> +
> +2012-09-04  Keishi Hattori  <keishi at webkit.org>

Three Change entries

> Source/WebCore/WebCore.exp.in:2574
> -#endif
> +#endif

Unrelated change

> Source/WebCore/html/shadow/CalendarPickerElement.h:71
> +    OwnPtr<DateTimeChooser> m_picker;

m_chooser is a better name.

> Source/WebKit/chromium/src/CalendarPickerPopup.h:48
> +class CalendarPickerPopup : public WebCore::DateTimeChooser, public
WebCore::PagePopupClient {

DateTiemChooserImpl might be a better class name.