New issue
Advanced search Search tips

Issue 695072 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: 2
NextAction: ----
OS: All
Pri: 2
Type: Bug
Proj-Servicification

Blocking:
issue 576261
issue 692157



Sign in to add a comment

PlzNavigate: preserve SourceLocation information

Project Member Reported by clamy@chromium.org, Feb 22 2017

Issue description

When PlzNavigate is enabled, we are losing the SourceLocation information when navigating, which results in error messages from the console missing the javascript line number. We should preserve this information somehow for Javascript-triggered navigations.
 
Blocking: 692157
Cc: carlosk@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19f01147eb8a5af46027bdbb1c843085fbac339c

commit 19f01147eb8a5af46027bdbb1c843085fbac339c
Author: clamy <clamy@chromium.org>
Date: Tue Mar 14 17:05:00 2017

PlzNavigate: preserve SourceLocation when navigating

This CL ensures we keep track of the SourceLocation information when
navigating, so that it can be used in console error messages generated
at commit time. The information will also be used in a later patch to
generate console error messages for CSP failures computed in the browser
process.

BUG= 695072 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2720763002
Cr-Commit-Position: refs/heads/master@{#456739}

[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/content/common/frame_messages.h
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/content/common/navigation_params.cc
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/content/common/navigation_params.h
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/content/public/test/render_view_test.cc
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/Source/core/frame/FrameConsole.cpp
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/Source/core/loader/DocumentLoader.h
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/Source/web/LocalFrameClientImpl.cpp
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/Source/web/WebDataSourceImpl.cpp
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/Source/web/WebDataSourceImpl.h
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/public/web/WebDataSource.h
[modify] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/public/web/WebFrameClient.h
[add] https://crrev.com/19f01147eb8a5af46027bdbb1c843085fbac339c/third_party/WebKit/public/web/WebSourceLocation.h

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a58df54d52b781b9628e7adee559157a8602150

commit 8a58df54d52b781b9628e7adee559157a8602150
Author: clamy <clamy@chromium.org>
Date: Mon Mar 20 15:27:19 2017

PlzNavigate: send SourceLocation when mixed content is found

This CL adds SourceLocation information to the IPC
FrameMsg_MixedContentFound so that Mixed Contents console error messages
display the appropriate line number. It also fixes an issue due to the
rebase of https://codereview.chromium.org/2720763002/, where the
SourceLocation information was not properly copied in the pending
navigation info in RenderFrameImpl.

BUG= 695072 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2745363004
Cr-Commit-Position: refs/heads/master@{#458068}

[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/content/browser/frame_host/mixed_content_navigation_throttle.cc
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/content/browser/frame_host/navigation_handle_impl.h
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/content/common/frame_messages.h
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/content/renderer/render_frame_impl.h
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/third_party/WebKit/Source/core/loader/MixedContentChecker.cpp
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/third_party/WebKit/Source/core/loader/MixedContentChecker.h
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/8a58df54d52b781b9628e7adee559157a8602150/third_party/WebKit/public/web/WebLocalFrame.h

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/54c5f4ac7707c8e5d5341eae008fa5f303f7656c

commit 54c5f4ac7707c8e5d5341eae008fa5f303f7656c
Author: dimich <dimich@chromium.org>
Date: Mon Mar 20 23:52:21 2017

Revert of PlzNavigate: send SourceLocation when mixed content is found (patchset #8 id:140001 of https://codereview.chromium.org/2745363004/ )

Reason for revert:
This enabled LayoutTests which started to fail:

https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/7963
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/7964
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/7965

There are text failures and a DCHECK:
14:48:10.177 5284 worker/2 virtual/mojo-loading/http/tests/serviceworker/chromium/request-body-blob-crash.html crashed, (stderr lines):
14:48:10.177 5284   [9083:38659:0320/144810.106602:2775098818460:FATAL:url_request.cc(1205)] Check failed: blocked_by_.empty().

Original issue's description:
> PlzNavigate: send SourceLocation when mixed content is found
>
> This CL adds SourceLocation information to the IPC
> FrameMsg_MixedContentFound so that Mixed Contents console error messages
> display the appropriate line number. It also fixes an issue due to the
> rebase of https://codereview.chromium.org/2720763002/, where the
> SourceLocation information was not properly copied in the pending
> navigation info in RenderFrameImpl.
>
> BUG= 695072 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
>
> Review-Url: https://codereview.chromium.org/2745363004
> Cr-Commit-Position: refs/heads/master@{#458068}
> Committed: https://chromium.googlesource.com/chromium/src/+/8a58df54d52b781b9628e7adee559157a8602150

TBR=nasko@chromium.org,japhet@chromium.org,jam@chromium.org,clamy@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 695072 

Review-Url: https://codereview.chromium.org/2765643002
Cr-Commit-Position: refs/heads/master@{#458245}

[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/content/browser/frame_host/mixed_content_navigation_throttle.cc
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/content/browser/frame_host/navigation_handle_impl.h
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/content/common/frame_messages.h
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/content/renderer/render_frame_impl.h
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/third_party/WebKit/Source/core/loader/MixedContentChecker.cpp
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/third_party/WebKit/Source/core/loader/MixedContentChecker.h
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/54c5f4ac7707c8e5d5341eae008fa5f303f7656c/third_party/WebKit/public/web/WebLocalFrame.h

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bca00eeaacad82d5d9d919e74ade9a76d533f99

commit 6bca00eeaacad82d5d9d919e74ade9a76d533f99
Author: dimich <dimich@chromium.org>
Date: Tue Mar 21 04:54:31 2017

Reland of PlzNavigate: send SourceLocation when mixed content is found (patchset #1 id:1 of https://codereview.chromium.org/2765643002/ )

Reason for revert:
Turns out this patch was not the reason for those failures. Re-landing it back.

Original issue's description:
> Revert of PlzNavigate: send SourceLocation when mixed content is found (patchset #8 id:140001 of https://codereview.chromium.org/2745363004/ )
>
> Reason for revert:
> This enabled LayoutTests which started to fail:
>
> https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/7963
> https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/7964
> https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/7965
>
> There are text failures and a DCHECK:
> 14:48:10.177 5284 worker/2 virtual/mojo-loading/http/tests/serviceworker/chromium/request-body-blob-crash.html crashed, (stderr lines):
> 14:48:10.177 5284   [9083:38659:0320/144810.106602:2775098818460:FATAL:url_request.cc(1205)] Check failed: blocked_by_.empty().
>
> Original issue's description:
> > PlzNavigate: send SourceLocation when mixed content is found
> >
> > This CL adds SourceLocation information to the IPC
> > FrameMsg_MixedContentFound so that Mixed Contents console error messages
> > display the appropriate line number. It also fixes an issue due to the
> > rebase of https://codereview.chromium.org/2720763002/, where the
> > SourceLocation information was not properly copied in the pending
> > navigation info in RenderFrameImpl.
> >
> > BUG= 695072 
> > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
> >
> > Review-Url: https://codereview.chromium.org/2745363004
> > Cr-Commit-Position: refs/heads/master@{#458068}
> > Committed: https://chromium.googlesource.com/chromium/src/+/8a58df54d52b781b9628e7adee559157a8602150
>
> TBR=nasko@chromium.org,japhet@chromium.org,jam@chromium.org,clamy@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 695072 
>
> Review-Url: https://codereview.chromium.org/2765643002
> Cr-Commit-Position: refs/heads/master@{#458245}
> Committed: https://chromium.googlesource.com/chromium/src/+/54c5f4ac7707c8e5d5341eae008fa5f303f7656c

TBR=nasko@chromium.org,japhet@chromium.org,jam@chromium.org,clamy@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 695072 

Review-Url: https://codereview.chromium.org/2760263002
Cr-Commit-Position: refs/heads/master@{#458311}

[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/content/browser/frame_host/mixed_content_navigation_throttle.cc
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/content/browser/frame_host/navigation_handle_impl.h
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/content/common/frame_messages.h
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/content/renderer/render_frame_impl.h
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/third_party/WebKit/LayoutTests/FlagExpectations/enable-browser-side-navigation
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/third_party/WebKit/Source/core/loader/MixedContentChecker.cpp
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/third_party/WebKit/Source/core/loader/MixedContentChecker.h
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/third_party/WebKit/Source/web/WebLocalFrameImpl.h
[modify] https://crrev.com/6bca00eeaacad82d5d9d919e74ade9a76d533f99/third_party/WebKit/public/web/WebLocalFrame.h

Comment 7 by clamy@chromium.org, Apr 20 2017

Status: Fixed (was: Started)

Comment 8 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 9 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Sign in to add a comment