Migrate addMessageToConsole API to RenderFrame.

Migrate addMessageToConsole API to RenderFrame.

BUG=304341
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261424

[mkosiba (inactive), 2014-03-28 18:47:34]

PTAL this depends on https://codereview.chromium.org/217283004/

[nasko, 2014-03-28 23:05:04]

Pretty close. Some comments.

https://codereview.chromium.org/217183004/diff/1/content/browser/frame_host/r...
File content/browser/frame_host/render_frame_host_delegate.h (right):

https://codereview.chromium.org/217183004/diff/1/content/browser/frame_host/r...
content/browser/frame_host/render_frame_host_delegate.h:31: virtual const GURL&
GetURL() const;
The name of this method should be adjusted to match what it will return. This
will be the URL for the main frame, which is not necessarily the frame the
called the delegate_.

Also, since this is implemented in WebContentsImpl, you should move the method
to the section for RenderFrameHostDelegate.

https://codereview.chromium.org/217183004/diff/1/content/common/frame_messages.h
File content/common/frame_messages.h (right):

https://codereview.chromium.org/217183004/diff/1/content/common/frame_message...
content/common/frame_messages.h:340: // WebKit and JavaScript error messages to
log to the console
nit: Blink

https://codereview.chromium.org/217183004/diff/1/content/public/renderer/rend...
File content/public/renderer/render_frame_observer.h (right):

https://codereview.chromium.org/217183004/diff/1/content/public/renderer/rend...
content/public/renderer/render_frame_observer.h:52: // Called when we receive a
console message from WebKit for which we requested
nit: Blink

https://codereview.chromium.org/217183004/diff/1/content/public/renderer/rend...
content/public/renderer/render_frame_observer.h:54: // |source| is the
WebKit-reported source of the error (either external or
nit: Blink

[mkosiba (inactive), 2014-03-31 09:48:40]

https://codereview.chromium.org/217183004/diff/1/content/browser/frame_host/r...
File content/browser/frame_host/render_frame_host_delegate.h (right):

https://codereview.chromium.org/217183004/diff/1/content/browser/frame_host/r...
content/browser/frame_host/render_frame_host_delegate.h:31: virtual const GURL&
GetURL() const;
On 2014/03/28 23:05:05, nasko wrote:
> The name of this method should be adjusted to match what it will return. This
> will be the URL for the main frame, which is not necessarily the frame the
> called the delegate_.
> 
> Also, since this is implemented in WebContentsImpl, you should move the method
> to the section for RenderFrameHostDelegate.

Done.

https://codereview.chromium.org/217183004/diff/1/content/common/frame_messages.h
File content/common/frame_messages.h (right):

https://codereview.chromium.org/217183004/diff/1/content/common/frame_message...
content/common/frame_messages.h:340: // WebKit and JavaScript error messages to
log to the console
On 2014/03/28 23:05:05, nasko wrote:
> nit: Blink

Done.

https://codereview.chromium.org/217183004/diff/1/content/public/renderer/rend...
File content/public/renderer/render_frame_observer.h (right):

https://codereview.chromium.org/217183004/diff/1/content/public/renderer/rend...
content/public/renderer/render_frame_observer.h:52: // Called when we receive a
console message from WebKit for which we requested
On 2014/03/28 23:05:05, nasko wrote:
> nit: Blink

Done.

https://codereview.chromium.org/217183004/diff/1/content/public/renderer/rend...
content/public/renderer/render_frame_observer.h:54: // |source| is the
WebKit-reported source of the error (either external or
On 2014/03/28 23:05:05, nasko wrote:
> nit: Blink

Done.

[nasko, 2014-03-31 13:45:02]

https://codereview.chromium.org/217183004/diff/40001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/217183004/diff/40001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_impl.cc:355:
HasWebUIScheme(delegate_->GetMainFrameVisibleURL()) ? level : 0;
The visible is usually the last committed, unless we have a slow navigation in
progress. If JS message is displayed on the console, there is no way it comes
from a navigation that is pending, it can only come from the current document.
So method shouldn't be using the VisibleURL, rather the LastCommittedURL.

[mkosiba (inactive), 2014-03-31 17:36:48]

https://codereview.chromium.org/217183004/diff/40001/content/browser/frame_ho...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/217183004/diff/40001/content/browser/frame_ho...
content/browser/frame_host/render_frame_host_impl.cc:355:
HasWebUIScheme(delegate_->GetMainFrameVisibleURL()) ? level : 0;
On 2014/03/31 13:45:02, nasko wrote:
> The visible is usually the last committed, unless we have a slow navigation in
> progress. If JS message is displayed on the console, there is no way it comes
> from a navigation that is pending, it can only come from the current document.
> So method shouldn't be using the VisibleURL, rather the LastCommittedURL.

Done.

[nasko, 2014-03-31 17:38:48]

LGTM

[mkosiba (inactive), 2014-04-01 11:20:30]

On 2014/03/31 17:38:48, nasko wrote:
> LGTM

Thanks for the review!

[jochen (gone - plz use gerrit), 2014-04-02 09:43:17]

lgtm

[mkosiba (inactive), 2014-04-02 10:25:25]

The CQ bit was checked by mkosiba@chromium.org

[commit-bot: I haz the power, 2014-04-02 10:25:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/217183004/60001

[commit-bot: I haz the power, 2014-04-02 10:57:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-02 10:57:48]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[mkosiba (inactive), 2014-04-02 12:42:05]

The CQ bit was checked by mkosiba@chromium.org

[commit-bot: I haz the power, 2014-04-02 12:42:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/217183004/80001

[commit-bot: I haz the power, 2014-04-02 17:22:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-02 17:22:37]

Retried try job too often on android_aosp for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_ao...

[mkosiba (inactive), 2014-04-03 09:29:46]

The CQ bit was checked by mkosiba@chromium.org

[commit-bot: I haz the power, 2014-04-03 09:30:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/217183004/80001

[commit-bot: I haz the power, 2014-04-03 10:41:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-03 10:41:45]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[mkosiba (inactive), 2014-04-03 10:58:49]

The CQ bit was checked by mkosiba@chromium.org

[commit-bot: I haz the power, 2014-04-03 10:59:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/217183004/80001

[commit-bot: I haz the power, 2014-04-03 11:04:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-03 11:04:16]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[mkosiba (inactive), 2014-04-03 11:13:45]

The CQ bit was checked by mkosiba@chromium.org

[commit-bot: I haz the power, 2014-04-03 11:13:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/217183004/80001

[commit-bot: I haz the power, 2014-04-03 14:59:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-03 14:59:21]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[mkosiba (inactive), 2014-04-03 15:45:57]

The CQ bit was checked by mkosiba@chromium.org

[commit-bot: I haz the power, 2014-04-03 15:49:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mkosiba@chromium.org/217183004/80001

[commit-bot: I haz the power, 2014-04-03 15:55:52]

Change committed as 261424