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

Issue 884773 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug
Proj-Servicification

Blocking:
issue 827532



Sign in to add a comment

ui_test_utils::NavigateToURL() doesn't seem to load title / content correctly with Network Service

Project Member Reported by chongz@chromium.org, Sep 17

Issue description

This issue applies to Chrome OS + --enable-features=NetworkService only.

How to Reproduce:
1. Compile with Chrome OS on Linux
2. out/osDefault/browser_tests --brave-new-test-launcher --enable-features=NetworkService --gtest_filter=BrowserTest.NoTitle

Sample Output:
```
[121320:121320:0917/100628.093162:WARNING:user_policy_manager_factory_chromeos.cc(208)] No policy loaded for known non-enterprise user
[121354:121364:0917/100628.115850:ERROR:network_service.cc(84)] Not implemented reached in std::unique_ptr<net::NetworkChangeNotifier> network::(anonymous namespace)::CreateNetworkChangeNotifierIfNeeded()
[121320:121320:0917/100628.329985:ERROR:gpu_interface_provider.cc(85)] Not implemented reached in virtual void content::GpuInterfaceProvider::RegisterOzoneGpuInterfaces(service_manager::BinderRegistry *)
../../chrome/browser/ui/browser_browsertest.cc:385: Failure
Expected equality of these values:
  LocaleWindowCaptionFromPageTitle(ASCIIToUTF16("title1.html"))
    Which is: Chromium - title1.html
  browser()->GetWindowTitleForCurrentTab( true )
    Which is: Chromium - file:///usr/local/google/home/chongz/source_code/chromium/src/chrome/test/data/title1.html
Stack trace:
#0 0x00000221b84c testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop()
#1 0x00000221b229 testing::internal::AssertHelper::operator=()
#2 0x0000012068a4 BrowserTest_NoTitle_Test::RunTestOnMainThread()
#3 0x00000543ae52 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#4 0x000004f0de4c ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#5 0x000004f0cd1a ChromeBrowserMainParts::PreMainMessageLoopRun()
#6 0x000003c4fc58 chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
#7 0x000002fe89e1 content::BrowserMainLoop::PreMainMessageLoopRun()
#8 0x000003493105 content::StartupTaskRunner::RunAllTasksNow()
#9 0x000002fe7127 content::BrowserMainLoop::CreateStartupTasks()
#10 0x000002feb263 content::BrowserMainRunnerImpl::Initialize()
#11 0x000002fe4c92 content::BrowserMain()
#12 0x000004ce769f content::ContentMainRunnerImpl::Run()
#13 0x000006f3fec5 service_manager::Main()
#14 0x000004ce5b34 content::ContentMain()
#15 0x00000543aa19 content::BrowserTestBase::SetUp()
#16 0x000004eaf2a5 InProcessBrowserTest::SetUp()
```

 
Labels: Proj-Servicification-Canary Hotlist-KnownIssue
Owner: chongz@chromium.org
Status: Started (was: Untriaged)
Navigation feels to be related to many test failures. Investigating this one first.

Cc: roc...@chromium.org jam@chromium.org
It seems that https://crrev.com/c/742652 added support for file:// URLs but test files were not whitelisted.

+rockot@ Do you have any preferences on how we should allow tests to run here?
e.g.
1. Move test files to temp folder before actually running tests (not sure if that's doable though).
2. Allow tests to turn off whitelist, or default whitelist off for tests?

Thanks!

---

White List file:
https://cs.chromium.org/chromium/src/chrome/browser/net/chrome_network_delegate.cc?l=89&rcl=1853b734e013824b9a25a24f172ccd6a15cdf2b6

Example paths for test files (Chrome OS on Linux):
```
path:/usr/local/blaa/chongz/path_to_chromium/src/chrome/test/data/title1.html
profile_path:/tmp/.org.chromium.Chromium.jrIlmI/dCN8Jly/user
```

Also, the test hits the following warning and fail
```
WARNING:simple_synchronous_entry.cc(1252)] Could not open platform files for entry.
```
if I replace the file:// URL with |embedded_test_server()->GetURL("/title1.html")|.

Yeah I would just go with #2. Can't the test fixture just call ChromeNetworkDelegate::EnableAccessToAllFilesForTesting(true)?
Re #c5: Thanks for the pointer! It might need some tweaks but I will make it work.
Can ChromeNetworkDelegate::IsAccessAllowed jus check g_access_to_all_files_enabled?
Re #c7: Thanks for the additional comment!
Yes I was planning to move the |g_access_to_all_files_enabled| check from |ChromeNetworkDelegate::OnCanAccessFile()| to |ChromeNetworkDelegate::IsAccessAllowed()| and see how it goes.
 Issue 884779  has been merged into this issue.
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 20

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

commit a7f5b321fab7a30566d5e799836f89a40916b3ec
Author: Chong Zhang <chongz@chromium.org>
Date: Thu Sep 20 00:05:24 2018

Allow tests to access all files on Chrome OS

Chrome OS browser tests uses |ChromeNetworkDelegate::IsAccessAllowed|
but |g_access_to_all_files_enabled| was not hooked in.

This patch moved |g_access_to_all_files_enabled| to
|IsAccessAllowedInternal| to make sure it's always checked.

Bug:  884773 
Change-Id: I5183f3250e13af151a987561bdf5f93a6f0200be
Reviewed-on: https://chromium-review.googlesource.com/1235122
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592617}
[modify] https://crrev.com/a7f5b321fab7a30566d5e799836f89a40916b3ec/chrome/browser/net/chrome_network_delegate.cc
[modify] https://crrev.com/a7f5b321fab7a30566d5e799836f89a40916b3ec/testing/buildbot/filters/mojo.fyi.chromeos.network_browser_tests.filter

Labels: M-71
Status: Fixed (was: Started)

Sign in to add a comment