Make NetErrorHelper a RenderFrameObserver.

Make NetErrorHelper a RenderFrameObserver.

Migrate from using/observing a RenderView (deprecated) to a RenderFrame. Only
construct a NetErrorHelper for the top-level RenderFrame for now since it's not
clear how a NetErrorHelper applied to a sub-frame is useful.

TEST=unit,trybot
BUG=none

[Elly Fong-Jones, 2014-01-14 18:13:17]

PTAL?

[nasko, 2014-01-14 19:22:15]

https://codereview.chromium.org/138583002/diff/1/chrome/renderer/chrome_conte...
File chrome/renderer/chrome_content_renderer_client.cc (right):

https://codereview.chromium.org/138583002/diff/1/chrome/renderer/chrome_conte...
chrome/renderer/chrome_content_renderer_client.cc:1015: NetErrorHelper* helper =
NetErrorHelper::Get(main_render_frame);
helper will be NULL for subframes, so you need to either check for NULL or check
if frame == main_render_frame.

[Elly Fong-Jones, 2014-01-14 19:31:59]

On 2014/01/14 19:22:15, nasko wrote:
>
https://codereview.chromium.org/138583002/diff/1/chrome/renderer/chrome_conte...
> File chrome/renderer/chrome_content_renderer_client.cc (right):
> 
>
https://codereview.chromium.org/138583002/diff/1/chrome/renderer/chrome_conte...
> chrome/renderer/chrome_content_renderer_client.cc:1015: NetErrorHelper* helper
=
> NetErrorHelper::Get(main_render_frame);
> helper will be NULL for subframes, so you need to either check for NULL or
check
> if frame == main_render_frame.

no it won't, because we don't get it from the subframes:

  content::RenderFrame* main_render_frame = render_view->GetMainRenderFrame();

Later on, when we don't always use the main frame, that might be broken though?

[nasko, 2014-01-14 19:49:49]

On 2014/01/14 19:31:59, Elly Jones wrote:
> On 2014/01/14 19:22:15, nasko wrote:
> >
>
https://codereview.chromium.org/138583002/diff/1/chrome/renderer/chrome_conte...
> > File chrome/renderer/chrome_content_renderer_client.cc (right):
> > 
> >
>
https://codereview.chromium.org/138583002/diff/1/chrome/renderer/chrome_conte...
> > chrome/renderer/chrome_content_renderer_client.cc:1015: NetErrorHelper*
helper
> =
> > NetErrorHelper::Get(main_render_frame);
> > helper will be NULL for subframes, so you need to either check for NULL or
> check
> > if frame == main_render_frame.
> 
> no it won't, because we don't get it from the subframes:
> 
>   content::RenderFrame* main_render_frame = render_view->GetMainRenderFrame();

duh! I misread the ::Get() to take the frame. Taking the main_render_frame is
fine.
 
> Later on, when we don't always use the main frame, that might be broken
though?

Yes, later on you want to start using the frame for which the error is. Soonish
there will be a way to get RenderFrame based on WebFrame, since those are 1:1.

[nasko, 2014-01-14 20:03:54]

LGTM

[jam, 2014-01-14 22:36:21]

lgtm

are you planning on removing the GetRenderView() calls in the observer as a
followup?

[Elly Fong-Jones, 2014-01-15 16:18:51]

On 2014/01/14 22:36:21, jam wrote:
> lgtm
> 
> are you planning on removing the GetRenderView() calls in the observer as a
> followup?

Yes.

[jam, 2014-01-16 16:17:22]

https://codereview.chromium.org/138583002/diff/1/chrome/renderer/chrome_conte...
File chrome/renderer/chrome_content_renderer_client.cc (right):

https://codereview.chromium.org/138583002/diff/1/chrome/renderer/chrome_conte...
chrome/renderer/chrome_content_renderer_client.cc:385: // condition to if
(!render_frame()->GetWebFrame()->parent())
note: this landed yesterday, so you can now do this