Updating PageClickTracker to Notify listeners When Text Input Loses Focus.

Updating PageClickTracker to Notify listeners When Text Input Loses Focus.

Changing PageClickTracker and PageClickListener to give listeners the option to be informed when a text input element loses focus after gaining it. This will be used by the new autofill UI code once it is moved out of WebKit.


BUG=51644
TEST=


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

[csharp, 2011-10-24 19:18:10]

Just adding the ability for the PageClickTracker class to inform listeners of
lost text input focus. Seemed like a good, small place to start the autofill
changes.

I think I've picked the right reviewers, but let me know if I didn't.

[Ilya Sherman, 2011-10-24 21:30:46]

Does this also work correctly if you tab into the text field, press the down
arrow to bring up the Autofill popup, and then click elsewhere on the page?

http://codereview.chromium.org/8351009/diff/1/chrome/renderer/page_click_trac...
File chrome/renderer/page_click_tracker.cc (right):

http://codereview.chromium.org/8351009/diff/1/chrome/renderer/page_click_trac...
chrome/renderer/page_click_tracker.cc:126: }
nit: No need for curly braces around a single line if-stmt.

http://codereview.chromium.org/8351009/diff/1/chrome/renderer/page_click_trac...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/1/chrome/renderer/page_click_trac...
chrome/renderer/page_click_tracker_browsertest.cc:81:
ASSERT_FALSE(button.isNull());
nit: If the |button| is not used, it should be safe to remove these two lines.

http://codereview.chromium.org/8351009/diff/1/chrome/renderer/page_click_trac...
chrome/renderer/page_click_tracker_browsertest.cc:84: PageClickTracker*
page_click_tracker_;
This should be a scoped_ptr<PageClickTracker>, so that memory is safely released
at the end of the test.  Better yet, can this just be a stack-allocated
PageClickTracker -- no pointers?

http://codereview.chromium.org/8351009/diff/1/chrome/renderer/page_click_trac...
chrome/renderer/page_click_tracker_browsertest.cc:147: // Click a text field and
test the no lost focus notifications are sent.
nit: "the" -> "that"

http://codereview.chromium.org/8351009/diff/1/chrome/renderer/page_click_trac...
chrome/renderer/page_click_tracker_browsertest.cc:169: 
Let's add a test immediately after this that clicking on a text field does not
fire off a notification, since the previously focused element was the button.

[csharp, 2011-10-25 19:35:06]

Changes made.

This should work for the case you listed because only mouse clicks can cause
page_click_tracker to execute anything.

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
chrome/renderer/page_click_tracker_browsertest.cc:65:
page_click_tracker_.reset(new PageClickTracker(view_));
This can't be changed because we need to wait for ChromeRenderViewTest::Setup()
because that is what sets view_. Since we disallow assignment as well we can't
just assign it here.

[Ilya Sherman, 2011-10-25 20:01:15]

On 2011/10/25 19:35:06, csharp wrote:
> Changes made.

Thanks.  For future reviews, please click the 'Done' button for each change made
-- it helps the reviewer to keep track of the review progress.

> This should work for the case you listed because only mouse clicks can
> cause page_click_tracker to execute anything.

I just tested this:
 (1) Load https://www.corp.google.com/~isherman/no_crawl/autofill/cc.html
 (2) Tab into the first form field
 (3) Press the down arrow to bring up the popup.
 (4) Click outside the field to dismiss the popup.

After step (4), InputElementLostFocus() was not called; presumably because
PageClickTracker's |last_node_clicked_| was null.

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
chrome/renderer/page_click_tracker_browsertest.cc:65:
page_click_tracker_.reset(new PageClickTracker(view_));
On 2011/10/25 19:35:06, csharp wrote:
> This can't be changed because we need to wait for
ChromeRenderViewTest::Setup()
> because that is what sets view_. Since we disallow assignment as well we can't
> just assign it here.

Ok, fair enough :)

[Ilya Sherman, 2011-10-25 20:06:16]

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
chrome/renderer/page_click_tracker_browsertest.cc:166: // Click on a text field
and ensure no lost focus notification is sent.
nit: "Click on a text field while the button has focus..."

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
chrome/renderer/page_click_tracker_browsertest.cc:172: // make sure only the
first listener is notified.
nit: "then a button" -> "then click a button"

[Ilya Sherman, 2011-10-25 20:11:50]

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
File chrome/renderer/page_click_tracker.cc (right):

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
chrome/renderer/page_click_tracker.cc:125: SelectedTextFieldLostFocus();
I think in order to handle the keyboard-then-mouse case, you'll want to examine
node.document().focusedNode(), rather than relying on last_node_clicked_ being
the focused node.

[csharp1, 2011-10-27 15:20:58]

Ok, I fixed the gaining focus with tab issue. 

I'm not sure if the way I handle the key press is the best way, but it was the
only way I could seem to get to work.

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
File chrome/renderer/page_click_tracker.cc (right):

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
chrome/renderer/page_click_tracker.cc:125: SelectedTextFieldLostFocus();
On 2011/10/25 20:11:50, Ilya Sherman wrote:
> I think in order to handle the keyboard-then-mouse case, you'll want to
examine
> node.document().focusedNode(), rather than relying on last_node_clicked_ being
> the focused node.

Done.

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
chrome/renderer/page_click_tracker_browsertest.cc:166: // Click on a text field
and ensure no lost focus notification is sent.
On 2011/10/25 20:06:16, Ilya Sherman wrote:
> nit: "Click on a text field while the button has focus..."

Done.

http://codereview.chromium.org/8351009/diff/5001/chrome/renderer/page_click_t...
chrome/renderer/page_click_tracker_browsertest.cc:172: // make sure only the
first listener is notified.
On 2011/10/25 20:06:16, Ilya Sherman wrote:
> nit: "then a button" -> "then click a button"

Done.

[Ilya Sherman, 2011-10-27 22:04:17]

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.cc (left):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.cc:112: last_node_clicked_.reset();
I think you might want to leave this in place.  Otherwise, I'm worried that
right clicks might trigger calls to InputElementClicked(), or something like
that.

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.cc (right):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.cc:125: SelectedTextFieldLostFocus();
nit: It might be cleaner to combine these two functions into a single one, named
something along the lines of CheckForSelectedFieldLosingFocus() (or something
more concise, if you have a better idea).

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.cc:135: was_focused_ =
(render_view()->GetFocusedNode() == last_node_clicked_);
nit: Just noticed this line, which uses |render_view()->GetFocusedNode()| rather
than |newly_clicked_node.document().focusedNode()|.  I think they're equivalent,
but we should probably pick one, for the sake of consistency.

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.h (right):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.h:59: bool
DidSelectedTextFieldLoseFocus(WebKit::WebNode newly_clicked_node);
nit: If you prefer to keep this method (see comment in .cc file), please move it
to an anonymous namespace in the .cc file.  We generally prefer to reduce the
visibility of an API as much as possible -- including moving it out of the
|private:| section into an anonymous namespace when we can.

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker_browsertest.cc:147: EXPECT_EQ(1,
SendKeyEvent(MockKeyboard::LAYOUT_UNITED_STATES,
It looks like you should not use this, as it is only implemented on Windows and
Linux.  Did you run into problems with trying to use a WebInputEvent?

[csharp, 2011-10-31 15:24:25]

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.cc (left):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.cc:112: last_node_clicked_.reset();
On 2011/10/27 22:04:17, Ilya Sherman wrote:
> I think you might want to leave this in place.  Otherwise, I'm worried that
> right clicks might trigger calls to InputElementClicked(), or something like
> that.

Done.

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.cc (right):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.cc:125: SelectedTextFieldLostFocus();
On 2011/10/27 22:04:17, Ilya Sherman wrote:
> nit: It might be cleaner to combine these two functions into a single one,
named
> something along the lines of CheckForSelectedFieldLosingFocus() (or something
> more concise, if you have a better idea).

Done.

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.cc:135: was_focused_ =
(render_view()->GetFocusedNode() == last_node_clicked_);
On 2011/10/27 22:04:17, Ilya Sherman wrote:
> nit: Just noticed this line, which uses |render_view()->GetFocusedNode()|
rather
> than |newly_clicked_node.document().focusedNode()|.  I think they're
equivalent,
> but we should probably pick one, for the sake of consistency.

Done.

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.h (right):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.h:59: bool
DidSelectedTextFieldLoseFocus(WebKit::WebNode newly_clicked_node);
On 2011/10/27 22:04:17, Ilya Sherman wrote:
> nit: If you prefer to keep this method (see comment in .cc file), please move
it
> to an anonymous namespace in the .cc file.  We generally prefer to reduce the
> visibility of an API as much as possible -- including moving it out of the
> |private:| section into an anonymous namespace when we can.

Done.

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker_browsertest.cc:147: EXPECT_EQ(1,
SendKeyEvent(MockKeyboard::LAYOUT_UNITED_STATES,
I couldn't figure out how to properly pass in a WebInputElement. The other
autofill test functions that use it just pass it directly to the tested class,
they don't seem to try and modify the actually focused element like we need to
here. Did I miss the way to do that?

On 2011/10/27 22:04:17, Ilya Sherman wrote:
> It looks like you should not use this, as it is only implemented on Windows
and
> Linux.  Did you run into problems with trying to use a WebInputEvent?

[Ilya Sherman, 2011-10-31 21:23:12]

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker_browsertest.cc:147: EXPECT_EQ(1,
SendKeyEvent(MockKeyboard::LAYOUT_UNITED_STATES,
On 2011/10/31 15:24:25, csharp wrote:
> I couldn't figure out how to properly pass in a WebInputElement. The other
> autofill test functions that use it just pass it directly to the tested class,
> they don't seem to try and modify the actually focused element like we need to
> here. Did I miss the way to do that?

You can do something like this: http://pastebin.com/radjKyx7.  I'm not sure if
there's a slicker approach.

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/autofill/pa...
File chrome/renderer/autofill/password_autofill_manager.h (right):

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/autofill/pa...
chrome/renderer/autofill/password_autofill_manager.h:75: virtual bool
InputElementLostFocus();
nit: Please annotate this function with OVERRIDE.  Long as you're here, might as
well annotate InputElementClicked as well.

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.cc (right):

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.cc:140: last_node_clicked_.reset();
nit: This is now redundant, since the |last_node_clicked_| is reset at the top
of the method.

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.h (right):

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.h:55: void HandleTextFieldLosingFocus(const
WebKit::WebNode& newly_clicked_node);
nit: "Handle" implies that the event being handled definitely happened.  That
is, in this case, the method name suggests that it will only be called when a
text field is losing focus.  Hence, I would prefer something like
"CheckForTextFieldLosingFocus", or "HandleTextFieldMaybeLosingFocus".

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker_browsertest.cc:66: // Rather than make it do
so for the test, we create a new object.
nit: The indentation here is slightly off.

[Ilya Sherman, 2011-10-31 21:25:21]

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/10001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker_browsertest.cc:147: EXPECT_EQ(1,
SendKeyEvent(MockKeyboard::LAYOUT_UNITED_STATES,
On 2011/10/31 21:23:12, Ilya Sherman wrote:
> On 2011/10/31 15:24:25, csharp wrote:
> > I couldn't figure out how to properly pass in a WebInputElement. The other
> > autofill test functions that use it just pass it directly to the tested
class,
> > they don't seem to try and modify the actually focused element like we need
to
> > here. Did I miss the way to do that?
> 
> You can do something like this: http://pastebin.com/radjKyx7.  I'm not sure if
> there's a slicker approach.

The other option is to implement SendKeyEvent on MacOS X and Aura, so that it
can safely be used in a cross-platform way.

[csharp, 2011-11-01 14:49:01]

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/autofill/pa...
File chrome/renderer/autofill/password_autofill_manager.h (right):

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/autofill/pa...
chrome/renderer/autofill/password_autofill_manager.h:75: virtual bool
InputElementLostFocus();
On 2011/10/31 21:23:12, Ilya Sherman wrote:
> nit: Please annotate this function with OVERRIDE.  Long as you're here, might
as
> well annotate InputElementClicked as well.

Done.

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.cc (right):

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.cc:140: last_node_clicked_.reset();
On 2011/10/31 21:23:12, Ilya Sherman wrote:
> nit: This is now redundant, since the |last_node_clicked_| is reset at the top
> of the method.

Done.

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker.h (right):

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker.h:55: void HandleTextFieldLosingFocus(const
WebKit::WebNode& newly_clicked_node);
On 2011/10/31 21:23:12, Ilya Sherman wrote:
> nit: "Handle" implies that the event being handled definitely happened.  That
> is, in this case, the method name suggests that it will only be called when a
> text field is losing focus.  Hence, I would prefer something like
> "CheckForTextFieldLosingFocus", or "HandleTextFieldMaybeLosingFocus".  

Done.

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/17001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker_browsertest.cc:66: // Rather than make it do
so for the test, we create a new object.
On 2011/10/31 21:23:12, Ilya Sherman wrote:
> nit: The indentation here is slightly off.

Done.

[Ilya Sherman, 2011-11-01 20:34:27]

LGTM.  Thanks for sticking it out on this one :)

[commit-bot: I haz the power, 2011-11-01 20:34:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/csharp@chromium.org/8351009/22001

[commit-bot: I haz the power, 2011-11-01 21:26:02]

Try job failure for 8351009-22001 (retry) on linux_rel for step "check_deps".
It's a second try, previously, step "check_deps" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[Ilya Sherman, 2011-11-01 21:38:02]

On 2011/11/01 21:26:02, I haz the power (commit-bot) wrote:
> Try job failure for 8351009-22001 (retry) on linux_rel for step "check_deps".

Illegal include: "content/common/view_messages.h"
    Because of "-content/common" from chrome/renderer's include_rules.
Illegal include: "content/renderer/render_view_impl.h"
    Because of "-content" from chrome's include_rules.

John, are these legitimate failures?  If so, what's the correct way to implement
this test?

[jam, 2011-11-02 19:03:14]

On 2011/11/01 21:38:02, Ilya Sherman wrote:
> On 2011/11/01 21:26:02, I haz the power (commit-bot) wrote:
> > Try job failure for 8351009-22001 (retry) on linux_rel for step
"check_deps".
> 
> Illegal include: "content/common/view_messages.h"
>     Because of "-content/common" from chrome/renderer's include_rules.
> Illegal include: "content/renderer/render_view_impl.h"
>     Because of "-content" from chrome's include_rules.
> 
> John, are these legitimate failures?  If so, what's the correct way to
implement
> this test?

sorry for the delay, just saw this

yes this is legitimate. view_messages is an internal detail of content, which is
why you can't use it in chrome. ChromeRenderViewTest derives from
RenderViewTest, so you can add a method to the latter to do this (last resort),
or you can try to look at using existing methods there like SendKeyEvent

[Ilya Sherman, 2011-11-02 21:52:05]

On 2011/11/02 19:03:14, John Abd-El-Malek wrote:
> On 2011/11/01 21:38:02, Ilya Sherman wrote:
> > On 2011/11/01 21:26:02, I haz the power (commit-bot) wrote:
> > > Try job failure for 8351009-22001 (retry) on linux_rel for step
> "check_deps".
> > 
> > Illegal include: "content/common/view_messages.h"
> >     Because of "-content/common" from chrome/renderer's include_rules.
> > Illegal include: "content/renderer/render_view_impl.h"
> >     Because of "-content" from chrome's include_rules.
> > 
> > John, are these legitimate failures?  If so, what's the correct way to
> implement
> > this test?
> 
> sorry for the delay, just saw this
> 
> yes this is legitimate. view_messages is an internal detail of content, which
is
> why you can't use it in chrome. ChromeRenderViewTest derives from
> RenderViewTest, so you can add a method to the latter to do this (last
resort),
> or you can try to look at using existing methods there like SendKeyEvent

Ok.  It looks like RenderViewTest::SendKeyEvent() is used only in
src/content/renderer/render_view_browsertest.cc, and
RenderViewTest::SendNativeKeyEvent() is used only in
src/content/renderer/render_view_browsertest_mac.mm.  It seems a little silly,
but we could also add RenderViewTest::SendWebKeyEvent(const
WebKit::WebKeyboardEvent& key_event), and update the implementation of
RenderViewTest::SendNativeKeyEvent() to just be """
SendWebkeyboardEvent(reinterpret_cast<WebKeyboardEvent>(key_event)) """.

The method that is currently named RenderViewTest::SendKeyEvent() should
probably be moved into content/renderer/render_view_browsertest.cc, but that's
probably a task for a separate follow-up CL.

[csharp, 2011-11-03 13:56:23]

Both of the existing functions don't seem like create picks, as SendKeyEvent
only works on Linux and Windows, and SendNativeKeyPress would require different
code for each OS.

So I like the idea of adding RenderViewTest::SendWebKeyEvent, use it to test
page_click_tracker and update RenderViewTest::SendNativeKeyEvent to use
SendWebKeyEvent. Does anyone have any problems with this plan?

[jam, 2011-11-03 16:55:33]

On 2011/11/03 13:56:23, csharp wrote:
> Both of the existing functions don't seem like create picks, as SendKeyEvent
> only works on Linux and Windows, and SendNativeKeyPress would require
different
> code for each OS.
> 
> So I like the idea of adding RenderViewTest::SendWebKeyEvent, use it to test
> page_click_tracker and update RenderViewTest::SendNativeKeyEvent to use
> SendWebKeyEvent. Does anyone have any problems with this plan?

sgtm

[csharp, 2011-11-03 17:39:11]

Ok, I've update the code so the SendWebKeyboardEvent is now a function in
RenderViewTest, and SendNativeKeyEvent call SendWebKeyboardEvent.

[Ilya Sherman, 2011-11-03 21:32:11]

LGTM with nit.  Feel free to tick the commit queue checkbox after you address
the nit.

http://codereview.chromium.org/8351009/diff/22001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/22001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker_browsertest.cc:13: #include
"testing/gtest/include/gtest/gtest.h"
nit: I think you wanted to remove the content/renderer/render_view_impl.h
include rather than this one.

[csharp, 2011-11-04 13:25:32]

http://codereview.chromium.org/8351009/diff/22001/chrome/renderer/page_click_...
File chrome/renderer/page_click_tracker_browsertest.cc (right):

http://codereview.chromium.org/8351009/diff/22001/chrome/renderer/page_click_...
chrome/renderer/page_click_tracker_browsertest.cc:13: #include
"testing/gtest/include/gtest/gtest.h"
On 2011/11/03 21:32:11, Ilya Sherman wrote:
> nit: I think you wanted to remove the content/renderer/render_view_impl.h
> include rather than this one.

Done.

[commit-bot: I haz the power, 2011-11-04 13:25:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/csharp@chromium.org/8351009/35001

[commit-bot: I haz the power, 2011-11-04 14:50:47]

Change committed as 108649