Requesting Review for BugFix Chromium:1935

Send the 'page_url' parameter of the page (on which the right click was made to open the context menu) as the 'referer' field of the http header field. This change is done for cases OPENLINKNEWTAB, OPENLINKNEWWINDOW, and NOT for OPENLINKOFFTHERECORD (Incognito).
Also Https to Https transition must NOT send the referrer and this is checked in the 'OpenURL' method.
Call to the ShouldHideReferrer method is done at render_view.cc

/* FIX for Issue 1935:  referer-header is not set when opening link in new tab via context menu  */

Contributed by Ramkumar Gokarnesan - ramgo@yahoo-inc.com

BUG=chromium:1935
TEST=Try right click on a URL and click 'open link in new tab' or 'open link in new window' and see the referer information being sent as a part of the http header field. verify with wireshark / VS debugger.

[Evan Stade, 2011-03-14 19:35:50]

This looks alright to me, but Brett might be a better reviewer

[brettw, 2011-03-14 19:51:37]

What should happen for iframes? It seems like the referrer should be the iframe
containing the link rather than the page URL. Or is that what this patch does?

http://codereview.chromium.org/6681030/diff/1/chrome/browser/tab_contents/ren...
File chrome/browser/tab_contents/render_view_context_menu.cc (right):

http://codereview.chromium.org/6681030/diff/1/chrome/browser/tab_contents/ren...
chrome/browser/tab_contents/render_view_context_menu.cc:1522: // Check security
policy to prevent referrer being sent
Should be indented 2 spaces.

http://codereview.chromium.org/6681030/diff/1/chrome/browser/tab_contents/ren...
chrome/browser/tab_contents/render_view_context_menu.cc:1524: bool hideReferrer
= WebSecurityPolicy::shouldHideReferrer(
Should be "hide_referrer"

[Ramkumar, 2011-03-15 05:44:55]

i havent touched the iframe case, can you please be specific with what has
to be done? possibly an example?  have fixed the other 2 you pointed out, ll
submit the fix now

On Tue, Mar 15, 2011 at 1:21 AM, <brettw@chromium.org> wrote:

> What should happen for iframes? It seems like the referrer should be the
> iframe
> containing the link rather than the page URL. Or is that what this patch
> does?
>
>
>
>
http://codereview.chromium.org/6681030/diff/1/chrome/browser/tab_contents/ren...
> File chrome/browser/tab_contents/render_view_context_menu.cc (right):
>
>
>
http://codereview.chromium.org/6681030/diff/1/chrome/browser/tab_contents/ren...
> chrome/browser/tab_contents/render_view_context_menu.cc:1522: // Check
>
> security policy to prevent referrer being sent
> Should be indented 2 spaces.
>
>
>
http://codereview.chromium.org/6681030/diff/1/chrome/browser/tab_contents/ren...
> chrome/browser/tab_contents/render_view_context_menu.cc:1524: bool
> hideReferrer = WebSecurityPolicy::shouldHideReferrer(
> Should be "hide_referrer"
>
>
> http://codereview.chromium.org/6681030/
>

[brettw, 2011-03-15 15:41:19]

I don't actually know what should be done. To test, just construct a page with
an iframe (or search for iframe examples on the web) and make sure that your
code has the same behavior with respect to the referrers.

[Ramkumar, 2011-03-16 06:19:40]

tried with this page 
http://nunzioweb.com/iframes-example.htm
 Referrers are being sent for the links present outside the iframes.
 When i tried a Wireshark analysis of packets for a "right click --> Open Link
in new tab" action i don't find any HTTP packets being sent at all! The same
behaviour exists with the original chromium, there are no HTTP packets, hence no
referrers! but a ctrl click on iframe send HTTP packets with correct referrer.

[Ramkumar, 2011-03-16 08:07:08]

Sorry it was taking it from the cache thats why there were no http packets sent
out. i'm studying the iframes behaviour, i'll get back to you

[Ramkumar, 2011-03-16 10:33:47]

for an iframe with link, what is the expected behaviour? this does NOT send the
url of the iframe with the link, it sends the page url, which had the iframe
itself.

[Ramkumar, 2011-03-16 11:45:46]

there is another parameter called 'frame_url' which has the subframe url where
the click was made, is there a way to check if the click was made on the page or
inside a frame in a page?

[Ramkumar, 2011-03-16 13:02:17]

have used the logic that if frame_url is empty, then send page_url, else send
frame_url, is that fine? if yes, i'll send that patch tomo.

[brettw, 2011-03-16 15:42:41]

On Wed, Mar 16, 2011 at 6:02 AM,  <ramkumar.gokarnesan@gmail.com> wrote:
> have used the logic that if frame_url is empty, then send page_url, else
> send
> frame_url, is that fine? if yes, i'll send that patch tomo.

Possibly, you should check what happens in the regular navigation case
to make sure this is the correct behavior.

Brett

[Ramkumar, 2011-03-17 06:39:32]

Thanks for pointing out, changed the behaviour as of Patch set 3. when a link
from a frame is right clicked --> open link in new tab or window, the frame's
source document's url will be sent as the referrer and if the link is on the
page itself, the page url will be sent as referrer. Hope this is fine.
checked with this site,
http://www.samisite.com/test-csb2nf/id43.htm

[Ramkumar, 2011-03-18 12:31:35]

please check the current code and tell me the further steps to follow!

[brettw, 2011-03-18 16:20:55]

I think your logic is good. I just have some indentation nitpicks.

Also, have you signed the CLA for contributing code? If not, see the links here:
http://www.chromium.org/developers/contributing-code for CLA (or the corporate
CCLA if you're working for a company).

http://codereview.chromium.org/6681030/diff/11002/chrome/browser/tab_contents...
File chrome/browser/tab_contents/render_view_context_menu.cc (right):

http://codereview.chromium.org/6681030/diff/11002/chrome/browser/tab_contents...
chrome/browser/tab_contents/render_view_context_menu.cc:1180:
OpenURL(params_.link_url,
You accidentally added some extra space before this line.

http://codereview.chromium.org/6681030/diff/11002/chrome/browser/tab_contents...
chrome/browser/tab_contents/render_view_context_menu.cc:1181:
params_.frame_url.is_empty() ? params_.page_url : params_.frame_url,
The indentation rule is align all arguments to after the "(" if possible (this
is how it used to be), or indent 4 extra spaces if you can't do that. In your
case, you don't have room, so this line should be indented 4 spaces from the
"Open" and then the other parameters below should be moved back to be aligned
with your new code.
      OpenURL(
          params_.link_url,
          params_.frame_url.is_empty() ? params_.page_url : params_.frame_url,
          source_tab_contents_->delegate()....

http://codereview.chromium.org/6681030/diff/11002/chrome/browser/tab_contents...
chrome/browser/tab_contents/render_view_context_menu.cc:1189:
OpenURL(params_.link_url,
The indentation here should match above.

http://codereview.chromium.org/6681030/diff/11002/chrome/browser/tab_contents...
chrome/browser/tab_contents/render_view_context_menu.cc:1195:
OpenURL(params_.link_url, GURL(),
I think this still fits on one line.

http://codereview.chromium.org/6681030/diff/11002/chrome/browser/tab_contents...
chrome/browser/tab_contents/render_view_context_menu.cc:1229:
params_.frame_url.is_empty() ? params_.page_url : params_.frame_url,
Same as above.

[Ramkumar, 2011-03-21 05:56:09]

Thanks again, all indentation done as required i guess, please give it a final
check, and reg the license, if a CCLA is signed, will only the company's name
appear in the AUTHORS file?

[brettw, 2011-03-26 17:54:18]

I can't check this in until I know what to add to the AUTHORS file. What name
should appear?

[Ramkumar, 2011-03-28 05:14:45]

I work for Yahoo!. The legal dept. is looking into the Corporate CLA. I'll get
back to you asap.

[darin (slow to review), 2011-04-13 16:38:32]

http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
File chrome/browser/tab_contents/render_view_context_menu.cc (right):

http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
chrome/browser/tab_contents/render_view_context_menu.cc:1529: bool hide_referrer
= WebSecurityPolicy::shouldHideReferrer(
we try to avoid using webkit on the main thread of the browser process.  can we
avoid doing so here as well?

won't webkit prune out the referrer automagically using the exact same
machinery?  maybe this code isn't necessary?

[brettw, 2011-04-13 17:43:20]

http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
File chrome/browser/tab_contents/render_view_context_menu.cc (right):

http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
chrome/browser/tab_contents/render_view_context_menu.cc:1529: bool hide_referrer
= WebSecurityPolicy::shouldHideReferrer(
I don't see this getting called in the new tab case in my testing, so we need to
add this call somewhere. Do we need a WebKit patch to add that, or is this call
here OK?

[Ramkumar, 2011-04-13 17:54:53]

That code WILL get called if you are going in an https page and opening an http
link, try giving a query at https://encrypted.google.com and click on a link and
see try using wireshark. that code will nullify the referrer. plus it is a
method in the 'public' folder that i'm using.( #include
"third_party/WebKit/Source/WebKit/chromium/public/WebSecurityPolicy.h") There
are similar headers included from public folder in other browser code too. so
i'm guessing it must be fine.
On 2011/04/13 17:43:20, brettw wrote:
>
http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
> File chrome/browser/tab_contents/render_view_context_menu.cc (right):
> 
>
http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
> chrome/browser/tab_contents/render_view_context_menu.cc:1529: bool
hide_referrer
> = WebSecurityPolicy::shouldHideReferrer(
> I don't see this getting called in the new tab case in my testing, so we need
to
> add this call somewhere. Do we need a WebKit patch to add that, or is this
call
> here OK?

[Ramkumar, 2011-04-13 17:55:50]

Also omitting that method call will send the referrer from https to http, which
is not acceptable.

On 2011/04/13 17:54:53, Ramkumar wrote:
> That code WILL get called if you are going in an https page and opening an
http
> link, try giving a query at https://encrypted.google.com and click on a link
and
> see try using wireshark. that code will nullify the referrer. plus it is a
> method in the 'public' folder that i'm using.( #include
> "third_party/WebKit/Source/WebKit/chromium/public/WebSecurityPolicy.h") There
> are similar headers included from public folder in other browser code too. so
> i'm guessing it must be fine.
> On 2011/04/13 17:43:20, brettw wrote:
> >
>
http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
> > File chrome/browser/tab_contents/render_view_context_menu.cc (right):
> > 
> >
>
http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
> > chrome/browser/tab_contents/render_view_context_menu.cc:1529: bool
> hide_referrer
> > = WebSecurityPolicy::shouldHideReferrer(
> > I don't see this getting called in the new tab case in my testing, so we
need
> to
> > add this call somewhere. Do we need a WebKit patch to add that, or is this
> call
> > here OK?

[darin (slow to review), 2011-04-13 23:30:43]

I agree with what you are trying to accomplish.

The concern with using WebKit code from the browser UI thread is complicated
:-/
WebKit is not really designed to be used from multiple threads, and in the
browser
process, the UI thread is not the WebKit main thread.  The WebKit main
thread is
actually a background thread in the browser process.  This is the
"in-process"
WebKit thread that you may have heard about.

Anyways, because WebKit is not really designed to be multi-threaded, it is
not
always clear which APIs are safe to use from "background" threads.  We will
need
to very carefully audit the code behind any WebKit API used on a background
thread.  In this case, I think it is super safe to use, but no where in the
WebKit API
is that contract enforced.  How will maintainers of WebKit know to preserve
that
contract?  It becomes risky business to use a WebKit API from a random
background
thread.

So, the usual rule of thumb is to avoid WebKit APIs like the plague when you
are on
the browser UI thread.

What to do about it in this case?  Well, you could add some code to
RenderView
where it handles this navigation request.  You could also look into
modifying code in
WebKit.  Why is it that WebKit allows this request to send an "illegal"
referrer?
Maybe it would be more robust to patch WebKit.

-Darin


On Wed, Apr 13, 2011 at 10:55 AM, <ramkumar.gokarnesan@gmail.com> wrote:

> Also omitting that method call will send the referrer from https to http,
> which
> is not acceptable.
>
>
> On 2011/04/13 17:54:53, Ramkumar wrote:
>
>> That code WILL get called if you are going in an https page and opening an
>>
> http
>
>> link, try giving a query at https://encrypted.google.com and click on a
>> link
>>
> and
>
>> see try using wireshark. that code will nullify the referrer. plus it is a
>> method in the 'public' folder that i'm using.( #include
>> "third_party/WebKit/Source/WebKit/chromium/public/WebSecurityPolicy.h")
>> There
>> are similar headers included from public folder in other browser code too.
>> so
>> i'm guessing it must be fine.
>> On 2011/04/13 17:43:20, brettw wrote:
>> >
>>
>
>
>
http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
>
>> > File chrome/browser/tab_contents/render_view_context_menu.cc (right):
>> >
>> >
>>
>
>
>
http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
>
>> > chrome/browser/tab_contents/render_view_context_menu.cc:1529: bool
>> hide_referrer
>> > = WebSecurityPolicy::shouldHideReferrer(
>> > I don't see this getting called in the new tab case in my testing, so we
>>
> need
>
>> to
>> > add this call somewhere. Do we need a WebKit patch to add that, or is
>> this
>> call
>> > here OK?
>>
>
>
>
> http://codereview.chromium.org/6681030/
>

[Ramkumar, 2011-04-14 06:00:52]

"Well, you could add some code to RenderView where it handles this navigation
request" --> This will duplicate the same logic again. Do you want me to
duplicate the logic of 'shouldHideReferrer' here to RenderView.cc? that will be
redundant 10 lines, but i can do that.
 "You could also look into modifying code in WebKit.  Why is it that WebKit
allows this request to send an "illegal" referrer?" -->  this is because for
this right click + open link in new tab action, the flow is NOT handled by
webkit code at all, thats the reason i had to use that call here. Its not a
problem with webkit, it handles the ctrl + click on a link flow and that is
handled fine with out any issues.
"In this case, I think it is super safe to use, but no where in the WebKit API
is that contract enforced.  How will maintainers of WebKit know to preserve that
contract?" --> Could you please tell me what exactly are you  referring to by
'contract'?

On 2011/04/13 23:30:43, 
darin wrote:

> I agree with what you are trying to accomplish.
> 
> The concern with using WebKit code from the browser UI thread is complicated
> :-/
> WebKit is not really designed to be used from multiple threads, and in the
> browser
> process, the UI thread is not the WebKit main thread.  The WebKit main
> thread is
> actually a background thread in the browser process.  This is the
> "in-process"
> WebKit thread that you may have heard about.
> 
> Anyways, because WebKit is not really designed to be multi-threaded, it is
> not
> always clear which APIs are safe to use from "background" threads.  We will
> need
> to very carefully audit the code behind any WebKit API used on a background
> thread.  In this case, I think it is super safe to use, but no where in the
> WebKit API
> is that contract enforced.  How will maintainers of WebKit know to preserve
> that
> contract?  It becomes risky business to use a WebKit API from a random
> background
> thread.
> 
> So, the usual rule of thumb is to avoid WebKit APIs like the plague when you
> are on
> the browser UI thread.
> 
> What to do about it in this case?  Well, you could add some code to
> RenderView
> where it handles this navigation request.  You could also look into
> modifying code in
> WebKit.  Why is it that WebKit allows this request to send an "illegal"
> referrer?
> Maybe it would be more robust to patch WebKit.
> 
> -Darin
> 
> 
> On Wed, Apr 13, 2011 at 10:55 AM, <mailto:ramkumar.gokarnesan@gmail.com>
wrote:
> 
> > Also omitting that method call will send the referrer from https to http,
> > which
> > is not acceptable.
> >
> >
> > On 2011/04/13 17:54:53, Ramkumar wrote:
> >
> >> That code WILL get called if you are going in an https page and opening an
> >>
> > http
> >
> >> link, try giving a query at https://encrypted.google.com and click on a
> >> link
> >>
> > and
> >
> >> see try using wireshark. that code will nullify the referrer. plus it is a
> >> method in the 'public' folder that i'm using.( #include
> >> "third_party/WebKit/Source/WebKit/chromium/public/WebSecurityPolicy.h")
> >> There
> >> are similar headers included from public folder in other browser code too.
> >> so
> >> i'm guessing it must be fine.
> >> On 2011/04/13 17:43:20, brettw wrote:
> >> >
> >>
> >
> >
> >
>
http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
> >
> >> > File chrome/browser/tab_contents/render_view_context_menu.cc (right):
> >> >
> >> >
> >>
> >
> >
> >
>
http://codereview.chromium.org/6681030/diff/19001/chrome/browser/tab_contents...
> >
> >> > chrome/browser/tab_contents/render_view_context_menu.cc:1529: bool
> >> hide_referrer
> >> > = WebSecurityPolicy::shouldHideReferrer(
> >> > I don't see this getting called in the new tab case in my testing, so we
> >>
> > need
> >
> >> to
> >> > add this call somewhere. Do we need a WebKit patch to add that, or is
> >> this
> >> call
> >> > here OK?
> >>
> >
> >
> >
> > http://codereview.chromium.org/6681030/
> >

[Ramkumar, 2011-04-14 10:53:39]

The shouldHideReferrer method just validates the referrer policy and doesn't
involve any state change in the webkit. Also, wouldn't it probably be better to
refactor that method out of the webkit?

[brettw, 2011-04-15 06:13:06]

> "Well, you could add some code to RenderView where it handles this navigation
> request" --> This will duplicate the same logic again. Do you want me to
> duplicate the logic of 'shouldHideReferrer' here to RenderView.cc? that will
be
> redundant 10 lines, but i can do that.

If Darin is OK with this approach, let's do this. If you're interested in
learning how to do WebKit commits, the next approach is great, but if you're not
it will add another step of process.

I think Darin is suggesting not copying the entire contents of the
shouldHideReferrer function, but the call to it. The code in RenderView is
guaranteed to be on the same thread as the rest of WebKit so there are no
concerns about using it.

>  "You could also look into modifying code in WebKit.  Why is it that WebKit
> allows this request to send an "illegal" referrer?" -->  this is because for
> this right click + open link in new tab action, the flow is NOT handled by
> webkit code at all, thats the reason i had to use that call here. Its not a
> problem with webkit, it handles the ctrl + click on a link flow and that is
> handled fine with out any issues.

Darin's that WebKit should ignore referrers explicitly set by the application
when they're illegal. This sounds like a good approach but is an extra step,
like I mentioned above.


> "In this case, I think it is super safe to use, but no where in the WebKit API
> is that contract enforced.  How will maintainers of WebKit know to preserve
that
> contract?" --> Could you please tell me what exactly are you  referring to by
> 'contract'?

He means the definition of what the function promises to the calling code. In
this case, whether it does certain calls into WebKit that are not threadsafe
(which is most of WebKit). For example, one could imagine this function being
changed in the future to check a global list of known protocols, and this could
introduce subtle threading bugs.

[Ramkumar, 2011-04-15 07:55:12]

I really didnt notice there was code flow through RenderView.cc. will it be
okay if i brought the shouldHideReferrer call somewhere there?

On Fri, Apr 15, 2011 at 11:43 AM, <brettw@chromium.org> wrote:

> "Well, you could add some code to RenderView where it handles this
>> navigation
>> request" --> This will duplicate the same logic again. Do you want me to
>> duplicate the logic of 'shouldHideReferrer' here to RenderView.cc? that
>> will
>>
> be
>
>> redundant 10 lines, but i can do that.
>>
>
> If Darin is OK with this approach, let's do this. If you're interested in
> learning how to do WebKit commits, the next approach is great, but if
> you're not
> it will add another step of process.
>
> I think Darin is suggesting not copying the entire contents of the
> shouldHideReferrer function, but the call to it. The code in RenderView is
> guaranteed to be on the same thread as the rest of WebKit so there are no
> concerns about using it.
>
>
>   "You could also look into modifying code in WebKit.  Why is it that
>> WebKit
>> allows this request to send an "illegal" referrer?" -->  this is because
>> for
>> this right click + open link in new tab action, the flow is NOT handled by
>> webkit code at all, thats the reason i had to use that call here. Its not
>> a
>> problem with webkit, it handles the ctrl + click on a link flow and that
>> is
>> handled fine with out any issues.
>>
>
> Darin's that WebKit should ignore referrers explicitly set by the
> application
> when they're illegal. This sounds like a good approach but is an extra
> step,
> like I mentioned above.
>
>
>
>  "In this case, I think it is super safe to use, but no where in the WebKit
>> API
>> is that contract enforced.  How will maintainers of WebKit know to
>> preserve
>>
> that
>
>> contract?" --> Could you please tell me what exactly are you  referring to
>> by
>> 'contract'?
>>
>
> He means the definition of what the function promises to the calling code.
> In
> this case, whether it does certain calls into WebKit that are not
> threadsafe
> (which is most of WebKit). For example, one could imagine this function
> being
> changed in the future to check a global list of known protocols, and this
> could
> introduce subtle threading bugs.
>
>
> http://codereview.chromium.org/6681030/
>

[Ramkumar, 2011-04-18 08:00:53]

Please review patch set 5, i have moved the logic to render_view.cc

[brettw, 2011-04-18 22:24:32]

LGTM with just a spacing nitpick. When you upload your final one, can you also
add the change to the AUTHORS file listing the name and email you should appear
as?

http://codereview.chromium.org/6681030/diff/31001/chrome/renderer/render_view.cc
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/6681030/diff/31001/chrome/renderer/render_view...
chrome/renderer/render_view.cc:1415: params.url,
These should be indented to only 4 spaces rather than randomly.

[Ramkumar, 2011-04-19 08:06:25]

Hey, think i fixed the 4 spaces indent, how can i add to the AUTHORS file? i
just get the file as a download! if you can add to it,
add the name as 'Ramkumar Gokarnesan' and e-mail as
'ramkumar.gokarnesan@yahoo.com'

On 2011/04/18 22:24:32, brettw wrote:
> LGTM with just a spacing nitpick. When you upload your final one, can you also
> add the change to the AUTHORS file listing the name and email you should
appear
> as?
> 
>
http://codereview.chromium.org/6681030/diff/31001/chrome/renderer/render_view.cc
> File chrome/renderer/render_view.cc (right):
> 
>
http://codereview.chromium.org/6681030/diff/31001/chrome/renderer/render_view...
> chrome/renderer/render_view.cc:1415: params.url,
> These should be indented to only 4 spaces rather than randomly.

[Ramkumar, 2011-04-19 08:32:51]

i have added this 
Ramkumar Gokarnesan - ramgo@yahoo-inc.com to the change list description please
have 'ramgo@yahoo-inc.com' as my mail id in the AUTHORS file. Many thanks.

[darin (slow to review), 2011-04-19 16:20:02]

On 2011/04/19 08:32:51, Ramkumar wrote:
> i have added this 
> Ramkumar Gokarnesan - mailto:ramgo@yahoo-inc.com to the change list
description please
> have 'ramgo@yahoo-inc.com' as my mail id in the AUTHORS file. Many thanks.

Hi Ramkumar,

Please add the change to the AUTHORS file to this patch.

Thanks!
-Darin

[darin (slow to review), 2011-04-20 06:13:57]

OK, LGTM

[commit-bot: I haz the power, 2011-04-20 06:14:19]

Can't apply patch for file AUTHORS.
patching file AUTHORS
Hunk #1 FAILED at 94.
1 out of 1 hunk FAILED -- saving rejects to file AUTHORS.rej

[Ramkumar, 2011-04-20 08:36:14]

Hey why this problem?! is the code committed?

On Wed, Apr 20, 2011 at 11:44 AM, <commit-bot@chromium.org> wrote:

> Can't apply patch for file AUTHORS.
> patching file AUTHORS
> Hunk #1 FAILED at 94.
> 1 out of 1 hunk FAILED -- saving rejects to file AUTHORS.rej
>
>
>
> http://codereview.chromium.org/6681030/
>

[darin (slow to review), 2011-04-20 16:13:12]

It suggests that someone else modified AUTHORS in the meantime.  If you "svn
update" and re-upload the patch, it should be good.  I can also try to land
your patch manually later today.

-Darin



On Wed, Apr 20, 2011 at 1:36 AM, Ramkumar Gokarnesan <
ramkumar.gokarnesan@gmail.com> wrote:

> Hey why this problem?! is the code committed?
>
>
> On Wed, Apr 20, 2011 at 11:44 AM, <commit-bot@chromium.org> wrote:
>
>> Can't apply patch for file AUTHORS.
>> patching file AUTHORS
>> Hunk #1 FAILED at 94.
>> 1 out of 1 hunk FAILED -- saving rejects to file AUTHORS.rej
>>
>>
>>
>> http://codereview.chromium.org/6681030/
>>
>
>

[brettw, 2011-04-20 17:30:41]

Checked in as r82323.

Your checkout was super old (looks like about a month) and there way this patch
would apply. I manually fixed the problems (mostly a result render_view.cc
moving). In the future, please be sure your checkout is up-to-date before
submitting.

Also, you didn't fix the indentation problem in render_view in the most recent
upload. I fixed this for you.

[Ramkumar, 2011-04-20 17:45:37]

Many thanks! Yeah, the fix was over long back, the license
verification took time, sorry about that. And i did change the
indentation to 4spaces from the function call, not from the margin.
Sorry! And thanks a lot for helping me out :)

On 4/20/11, brettw@chromium.org <brettw@chromium.org> wrote:
> Checked in as r82323.
>
> Your checkout was super old (looks like about a month) and there way this
> patch
> would apply. I manually fixed the problems (mostly a result render_view.cc
> moving). In the future, please be sure your checkout is up-to-date before
> submitting.
>
> Also, you didn't fix the indentation problem in render_view in the most
> recent
> upload. I fixed this for you.
>
> http://codereview.chromium.org/6681030/
>

-- 
Sent from my mobile device