Switch to WebFrame from the WebKit API....

Switch to WebFrame from the WebKit API.

I tried to avoid unnecessary changes in this CL to help make it easier to
review.

As part of this CL, glue/webtextinput* are folded into WebFrame / WebFrameImpl.

R=dglazkov
BUG=10034
TEST=none


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

[darin (slow to review), 2009-08-09 19:49:30]

Sorry for the huge CL.  I added comments for the interesting things.  The rest
should be pretty mechanical.

http://codereview.chromium.org/164225/diff/2388/3242
File chrome/common/resource_dispatcher_unittest.cc (left):

http://codereview.chromium.org/164225/diff/2388/3242#oldcode137
Line 137: static ResourceDispatcher* GetResourceDispatcher(WebFrame* unused) {
this function was unused, so i deleted it.

http://codereview.chromium.org/164225/diff/2388/3261
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/164225/diff/2388/3261#newcode2550
Line 2550: WebFrame* RenderView::GetChildFrame(const std::wstring& xpath) const
{
previously, WebFrame::GetChildFrame would handle nested xpaths, but i decided to
not support that in findChildByExpression.  i did that for consistency with
findChildByName which only looks for immediate children of the current frame. 
anyways, this nested xpath syntax is very specific to chrome's automation system
and UI tests.

http://codereview.chromium.org/164225/diff/2388/3265
File webkit/api/public/WebFrame.h (right):

http://codereview.chromium.org/164225/diff/2388/3265#newcode298
Line 298: virtual void insertText(const WebString& text) = 0;
these methods come from the old WebTextInput interface, which i'm just merging
into WebFrame.

http://codereview.chromium.org/164225/diff/2388/3265#newcode324
Line 324: virtual WebString selectionAsText() const = 0;
these methods used to be one method: WebFrame::GetSelection(bool as_html)

http://codereview.chromium.org/164225/diff/2388/3265#newcode346
Line 346: // Searches a frame for a given string.
i decided not to change the find in page API at all for this round.  i may
circle back and revise it in the future.  i think the method names at least
could use some cleanup (to make them more self consistent).

http://codereview.chromium.org/164225/diff/2388/3265#newcode418
Line 418: virtual WebString contentAsText(size_t maxChars) const = 0;
was originally WebFrame::GetContentAsPlainText

http://codereview.chromium.org/164225/diff/2388/3265#newcode420
Line 420: virtual WebString contentAsMarkup() const = 0;
was originally WebFrame::GetFullPageHtml

http://codereview.chromium.org/164225/diff/2388/3263
File webkit/api/public/WebRange.h (right):

http://codereview.chromium.org/164225/diff/2388/3263#newcode36
Line 36: #if WEBKIT_IMPLEMENTATION
the changes here are very mechanical.  i'm just enabling WebRange in this CL. 
see WebNode for code that follows a very similar pattern.

http://codereview.chromium.org/164225/diff/2388/3264
File webkit/api/public/WebString.h (right):

http://codereview.chromium.org/164225/diff/2388/3264#newcode80
Line 80: WEBKIT_API WebCString utf8() const;
this simplifies a lot of consumer code.  i plan to make a second pass to see if
i can't make use of this more.  i also need to add a similar .utf16() method on
WebCString, but i decided not to include that in this CL since this CL was
already large enough.

http://codereview.chromium.org/164225/diff/2388/3286
File webkit/glue/webframe_impl.cc (right):

http://codereview.chromium.org/164225/diff/2388/3286#newcode1608
Line 1608: bool WebFrameImpl::executeCommand(const WebString& name) {
this comes from webtextinput_impl.cc

http://codereview.chromium.org/164225/diff/2388/3307
File webkit/glue/webview_impl.cc (right):

http://codereview.chromium.org/164225/diff/2388/3307#newcode823
Line 823: GetFocusedFrame()->executeCommand(WebString::fromUTF8("Copy"));
there were a bunch of helper functions on WebFrame that just equate to an
execCommand.  so i decided to make poeple use executeCommand directly.

http://codereview.chromium.org/164225/diff/2388/3316
File webkit/tools/test_shell/test_webview_delegate_win.cc (right):

http://codereview.chromium.org/164225/diff/2388/3316#newcode162
Line 162: show(WebKit::WebNavigationPolicyNewWindow);
this fixes a leftover todo from a previous change of mine.  oops!

http://codereview.chromium.org/164225/diff/2388/3319
File webkit/tools/test_shell/text_input_controller.cc (right):

http://codereview.chromium.org/164225/diff/2388/3319#newcode112
Line 112: result->SetNull();
i decided to just mark these as not implemented instead of plumbing them
needlessly.

http://codereview.chromium.org/164225/diff/2388/3319#newcode138
Line 138: SStringPrintf(&range_str, "%d,%d", range.startOffset(),
range.endOffset());
this string formatting used to live inside webtextinput_impl.cc ... i think it
is better to have it in test_shell instead.  that's why i need WebRange :)

[darin (slow to review), 2009-08-09 19:52:29]

I uploaded a few more minor tweaks.  The comments I wrote below are against
patch set 16, so you might want to review that one instead.
-Darin



On Sun, Aug 9, 2009 at 12:49 PM, <darin@chromium.org> wrote:

> Reviewers: Dimitri Glazkov,
>
> Message:
> Sorry for the huge CL.  I added comments for the interesting things.
> The rest should be pretty mechanical.
>
>
> http://codereview.chromium.org/164225/diff/2388/3242
> File chrome/common/resource_dispatcher_unittest.cc (left):
>
> http://codereview.chromium.org/164225/diff/2388/3242#oldcode137
> Line 137: static ResourceDispatcher* GetResourceDispatcher(WebFrame*
> unused) {
> this function was unused, so i deleted it.
>
> http://codereview.chromium.org/164225/diff/2388/3261
> File chrome/renderer/render_view.cc (right):
>
> http://codereview.chromium.org/164225/diff/2388/3261#newcode2550
> Line 2550: WebFrame* RenderView::GetChildFrame(const std::wstring&
> xpath) const {
> previously, WebFrame::GetChildFrame would handle nested xpaths, but i
> decided to not support that in findChildByExpression.  i did that for
> consistency with findChildByName which only looks for immediate children
> of the current frame.  anyways, this nested xpath syntax is very
> specific to chrome's automation system and UI tests.
>
> http://codereview.chromium.org/164225/diff/2388/3265
> File webkit/api/public/WebFrame.h (right):
>
> http://codereview.chromium.org/164225/diff/2388/3265#newcode298
> Line 298: virtual void insertText(const WebString& text) = 0;
> these methods come from the old WebTextInput interface, which i'm just
> merging into WebFrame.
>
> http://codereview.chromium.org/164225/diff/2388/3265#newcode324
> Line 324: virtual WebString selectionAsText() const = 0;
> these methods used to be one method: WebFrame::GetSelection(bool
> as_html)
>
> http://codereview.chromium.org/164225/diff/2388/3265#newcode346
> Line 346: // Searches a frame for a given string.
> i decided not to change the find in page API at all for this round.  i
> may circle back and revise it in the future.  i think the method names
> at least could use some cleanup (to make them more self consistent).
>
> http://codereview.chromium.org/164225/diff/2388/3265#newcode418
> Line 418: virtual WebString contentAsText(size_t maxChars) const = 0;
> was originally WebFrame::GetContentAsPlainText
>
> http://codereview.chromium.org/164225/diff/2388/3265#newcode420
> Line 420: virtual WebString contentAsMarkup() const = 0;
> was originally WebFrame::GetFullPageHtml
>
> http://codereview.chromium.org/164225/diff/2388/3263
> File webkit/api/public/WebRange.h (right):
>
> http://codereview.chromium.org/164225/diff/2388/3263#newcode36
> Line 36: #if WEBKIT_IMPLEMENTATION
> the changes here are very mechanical.  i'm just enabling WebRange in
> this CL.  see WebNode for code that follows a very similar pattern.
>
> http://codereview.chromium.org/164225/diff/2388/3264
> File webkit/api/public/WebString.h (right):
>
> http://codereview.chromium.org/164225/diff/2388/3264#newcode80
> Line 80: WEBKIT_API WebCString utf8() const;
> this simplifies a lot of consumer code.  i plan to make a second pass to
> see if i can't make use of this more.  i also need to add a similar
> .utf16() method on WebCString, but i decided not to include that in this
> CL since this CL was already large enough.
>
> http://codereview.chromium.org/164225/diff/2388/3286
> File webkit/glue/webframe_impl.cc (right):
>
> http://codereview.chromium.org/164225/diff/2388/3286#newcode1608
> Line 1608: bool WebFrameImpl::executeCommand(const WebString& name) {
> this comes from webtextinput_impl.cc
>
> http://codereview.chromium.org/164225/diff/2388/3307
> File webkit/glue/webview_impl.cc (right):
>
> http://codereview.chromium.org/164225/diff/2388/3307#newcode823
> Line 823:
> GetFocusedFrame()->executeCommand(WebString::fromUTF8("Copy"));
> there were a bunch of helper functions on WebFrame that just equate to
> an execCommand.  so i decided to make poeple use executeCommand
> directly.
>
> http://codereview.chromium.org/164225/diff/2388/3316
> File webkit/tools/test_shell/test_webview_delegate_win.cc (right):
>
> http://codereview.chromium.org/164225/diff/2388/3316#newcode162
> Line 162: show(WebKit::WebNavigationPolicyNewWindow);
> this fixes a leftover todo from a previous change of mine.  oops!
>
> http://codereview.chromium.org/164225/diff/2388/3319
> File webkit/tools/test_shell/text_input_controller.cc (right):
>
> http://codereview.chromium.org/164225/diff/2388/3319#newcode112
> Line 112: result->SetNull();
> i decided to just mark these as not implemented instead of plumbing them
> needlessly.
>
> http://codereview.chromium.org/164225/diff/2388/3319#newcode138
> Line 138: SStringPrintf(&range_str, "%d,%d", range.startOffset(),
> range.endOffset());
> this string formatting used to live inside webtextinput_impl.cc ... i
> think it is better to have it in test_shell instead.  that's why i need
> WebRange :)
>
> Description:
> Switch to WebFrame from the WebKit API.
>
> I tried to avoid unnecessary changes in this CL to help make it easier
> to
> review.
>
> As part of this CL, glue/webtextinput* are folded into WebFrame /
> WebFrameImpl.
>
> R=dglazkov
> BUG=10034
> TEST=none
>
>
> Please review this at http://codereview.chromium.org/164225
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     chrome/common/resource_dispatcher_unittest.cc
>  M     chrome/renderer/extensions/bindings_utils.h
>  M     chrome/renderer/extensions/bindings_utils.cc
>  M     chrome/renderer/extensions/event_bindings.h
>  M     chrome/renderer/extensions/event_bindings.cc
>  M     chrome/renderer/extensions/extension_process_bindings.cc
>  MM    chrome/renderer/external_extension.cc
>  M     chrome/renderer/external_host_bindings.h
>  M     chrome/renderer/external_host_bindings.cc
>  M     chrome/renderer/loadtimes_extension_bindings.cc
>  M     chrome/renderer/print_web_view_helper.h
>  M     chrome/renderer/print_web_view_helper.cc
>  MM    chrome/renderer/print_web_view_helper_linux.cc
>  M     chrome/renderer/print_web_view_helper_mac.cc
>  M     chrome/renderer/print_web_view_helper_win.cc
>  M     chrome/renderer/render_view.h
>  M     chrome/renderer/render_view.cc
>  M     chrome/renderer/render_view_unittest.cc
>  M     chrome/renderer/renderer_glue.cc
>  M     chrome/renderer/user_script_slave.h
>  M     chrome/renderer/user_script_slave.cc
>  M     chrome/renderer/webplugin_delegate_proxy.cc
>  MM    chrome/test/render_view_test.h
>  M     chrome/test/render_view_test.cc
>  M     webkit/api/public/WebFrame.h
>  M     webkit/api/public/WebRange.h
>  M     webkit/api/public/WebString.h
>  A     webkit/api/src/WebRange.cpp
>  M     webkit/api/src/WebString.cpp
>  M     webkit/glue/alt_error_page_resource_fetcher.h
>  M     webkit/glue/alt_error_page_resource_fetcher.cc
>  M     webkit/glue/chrome_client_impl.cc
>  M     webkit/glue/context_menu_unittest.cc
>  M     webkit/glue/cpp_bound_class.h
>  M     webkit/glue/cpp_bound_class.cc
>  M     webkit/glue/cpp_bound_class_unittest.cc
>  M     webkit/glue/devtools/devtools_rpc_js.h
>  M     webkit/glue/devtools/dom_agent_unittest.cc
>  M     webkit/glue/dom_operations.cc
>  M     webkit/glue/dom_operations_unittest.cc
>  M     webkit/glue/dom_serializer.h
>  M     webkit/glue/dom_serializer.cc
>  M     webkit/glue/dom_serializer_unittest.cc
>  M     webkit/glue/editor_client_impl.cc
>  M     webkit/glue/glue_util.h
>  M     webkit/glue/glue_util.cc
>  M     webkit/glue/iframe_redirect_unittest.cc
>  M     webkit/glue/image_resource_fetcher.h
>  M     webkit/glue/image_resource_fetcher.cc
>  M     webkit/glue/inspector_client_impl.cc
>  M     webkit/glue/mimetype_unittest.cc
>  M     webkit/glue/resource_fetcher.h
>  M     webkit/glue/resource_fetcher.cc
>  M     webkit/glue/resource_fetcher_unittest.cc
>  M     webkit/glue/resource_loader_bridge.h
>  M     webkit/glue/webdevtoolsagent_impl.h
>  M     webkit/glue/webdevtoolsagent_impl.cc
>  M     webkit/glue/webdevtoolsclient_impl.cc
>  D     webkit/glue/webframe.h
>  M     webkit/glue/webframe_impl.h
>  M     webkit/glue/webframe_impl.cc
>  M     webkit/glue/webframe_unittest.cc
>  M     webkit/glue/webframeloaderclient_impl.cc
>  M     webkit/glue/webkit_glue.h
>  M     webkit/glue/webkit_glue.cc
>  M     webkit/glue/webplugin.h
>  M     webkit/glue/webplugin_impl.cc
>  D     webkit/glue/webtextinput.h
>  D     webkit/glue/webtextinput_impl.h
>  D     webkit/glue/webtextinput_impl.cc
>  M     webkit/glue/webview.h
>  M     webkit/glue/webview_delegate.h
>  M     webkit/glue/webview_impl.h
>  M     webkit/glue/webview_impl.cc
>  M     webkit/glue/webworker_impl.cc
>  M     webkit/tools/test_shell/layout_test_controller.cc
>  M     webkit/tools/test_shell/plugin_tests.cc
>  M     webkit/tools/test_shell/test_shell.h
>  M     webkit/tools/test_shell/test_shell.cc
>  M     webkit/tools/test_shell/test_shell_gtk.cc
>  M     webkit/tools/test_shell/test_shell_mac.mm
>  M     webkit/tools/test_shell/test_shell_win.cc
>  M     webkit/tools/test_shell/test_webview_delegate.h
>  M     webkit/tools/test_shell/test_webview_delegate.cc
>  M     webkit/tools/test_shell/test_webview_delegate_gtk.cc
>  M     webkit/tools/test_shell/test_webview_delegate_win.cc
>  M     webkit/tools/test_shell/text_input_controller.h
>  M     webkit/tools/test_shell/text_input_controller.cc
>  M     webkit/webkit.gyp
>
>
>

[dglazkov, 2009-08-09 21:47:03]

LGTM. I assume this will be landing under the covers of darkness sometime
tonight? ;)

[michaeln, 2009-08-10 03:25:43]

Ah... fantastic... was wondering when WebKit::WebFrame might come online.