New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 709431 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

PlzNavigate: Access violation ContentSerializedNavigationBuilder::FromNavigationEntry

Project Member Reported by arthurso...@chromium.org, Apr 7 2017

Issue description

This bug occurs only with the PlzNavigate experiment --enable-browser-side-navigation
An invalid reference is passed to ContentSerializedNavigationBuilder::FromNavigationEntry(...)

Crash Link
https://crash.corp.google.com/browse?q=product.name%3D%22Chrome_Android%22%20%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%22sessions%3A%3AContentSerializedNavigationBuilder%3A%3AFromNavigationEntry%22%20OMIT%20RECORD%20IF%20SUM(custom_data.ChromeCrashProto.experiments.ids%3D%22c68ab9a3-6edc92c7%22)%20%3D%200&ignore_case=false&enable_rewrite=false&omit_field_name=&omit_field_value=&omit_field_opt=

Stack:
0x0000007f665a54dc	(libchrome.so -content_serialized_navigation_builder.cc:28 )	sessions::ContentSerializedNavigationBuilder::FromNavigationEntry(int, content::NavigationEntry const&)
0x0000007f66b334ec	(libchrome.so -tab_state.cc:348 )	WebContentsState::WriteNavigationsAsByteBuffer(_JNIEnv*, bool, std::__ndk1::vector<content::NavigationEntry*, std::__ndk1::allocator<content::NavigationEntry*> > const&, int)
0x0000007f66b33764	(libchrome.so -tab_state.cc:319 )	WebContentsState::GetContentsStateAsByteBuffer(_JNIEnv*, TabAndroid*)
0x0000007f66b337d4	(libchrome.so -tab_state.cc:525 )	Java_org_chromium_chrome_browser_TabState_nativeGetContentsStateAsByteBuffer
0x0000007f6a596a10	(data@app@com.chrome.dev-2@base.apk@classes.dex + 0x004bba10 )	
 
Labels: Proj-PlzNavigate OS-Android
Status: Started (was: Available)
Started and it should be fixed by:
https://codereview.chromium.org/2802213002/
https://codereview.chromium.org/2810173002/
Issue 446839 has been merged into this issue.
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 14 2017

Labels: FoundIn-M-59 Fracas
Users experienced this crash on the following builds:

Android Dev 59.0.3062.4 -  2.04 CPM, 50 reports, 47 clients (signature sessions::ContentSerializedNavigationBuilder::FromNavigationEntry)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

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

Labels: Proj-PlzNavigate-Blocking
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 25 2017

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

commit 5c4c202d728cacf71c4ac518e9ab5ee8440eafcf
Author: arthursonzogni <arthursonzogni@chromium.org>
Date: Tue Apr 25 23:41:27 2017

This CL makes that the following invariant stay true:

If   (pending_entry_index != -1)
Then (pending_entry_ = entries[pending_entry_index_])

There were several parts of the code that make it become false.

The bug was discovered thanks to
[Add DCHECKs for invariants on NavigationController indices.]
(https://codereview.chromium.org/2815753003/)

BUG= 709431 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation;master.tryserver.chromium.linux:linux_chromium_browser_side_navigation_rel

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

[modify] https://crrev.com/5c4c202d728cacf71c4ac518e9ab5ee8440eafcf/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/5c4c202d728cacf71c4ac518e9ab5ee8440eafcf/content/browser/frame_host/navigation_controller_impl_unittest.cc
[modify] https://crrev.com/5c4c202d728cacf71c4ac518e9ab5ee8440eafcf/content/public/browser/navigation_controller.h

Status: Fixed (was: Started)
The latest CL landed in 60.0.3081.0
I hoped it would fix the bug and I don't see anymore crash reports after this.
I guess the problem is resolved.
Project Member

Comment 8 by bugdroid1@chromium.org, May 3 2017

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

commit 02084c771e830293de3cc4919d29a83341558035
Author: arthursonzogni <arthursonzogni@chromium.org>
Date: Wed May 03 22:52:39 2017

Simplify WebContentsState::GetContentsStateAsByteBuffer

This CL remove some conditions that must never be true:

* (entry_count == 0 && pending_index ==  0) because
  (entry_count == 0 => pending_index == -1)

* (current_index == -1 && entry_count >  0) because
  (current_index == -1 => entry_count == 0)

A similar simplification can be found in:
https://codereview.chromium.org/2762363002/

This CL was done while trying to understand the crashes in:
 https://crbug.com/709431 

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

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

[modify] https://crrev.com/02084c771e830293de3cc4919d29a83341558035/chrome/browser/android/tab_state.cc
[modify] https://crrev.com/02084c771e830293de3cc4919d29a83341558035/content/browser/frame_host/navigation_controller_impl.h

Sign in to add a comment