New issue
Advanced search Search tips

Issue 845455 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocking:
issue 843511



Sign in to add a comment

Several extensions_browsertests don't pass on mac in component builds

Project Member Reported by thakis@chromium.org, May 22 2018

Issue description

I'm adding more tests to bots. I added extensions_browsertests to the clang to bot ToTMac, which does component builds. Several tests fail, some like so: 

https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.clang%2FToTMac%2F1649%2F%2B%2Frecipes%2Fsteps%2Fextensions_browsertests%2F0%2Flogs%2FPrinterProviderApiTest.PrintJobFailed%2F0

https://ci.chromium.org/buildbot/chromium.clang/ToTMac/1649

dyld: Library not loaded: @rpath/libcontent.dylib
  Referenced from: /b/s/w/ir/out/Release/App Shell.app/Contents/Frameworks/App Shell Helper.app/Contents/MacOS/App Shell Helper
  Reason: image not found


Sounds like 'App Shell Helper.app' lacks the rpath adjustment that the other helper apps have, e.g. here: https://cs.chromium.org/chromium/src/content/shell/BUILD.gn?type=cs&q=rpath+file:%5C.gn+file:content&sq=package:chromium&g=0&l=760

    if (is_component_build) {
      # Set up the rpath for the framework so that it can find dylibs in the
      # root output directory. The framework is at
      # Content Shell.app/Contents/Frameworks/Content Shell Framework.framework/Versions/C/Content Shell Framework
      # so use loader_path to go back to the root output directory.
      ldflags += [
        "-rpath",
        "@loader_path/../../../../../..",
      ]
    }


(This is similar to  issue 845451  which is the same bug in a different executable)
 

Comment 1 by tapted@chromium.org, May 23 2018

Cc: rdevlin....@chromium.org michae...@chromium.org
Components: -Platform>Apps Platform>Extensions Platform>Apps>Shell
Ah, App Shell Helper.app.. Based on comments in http://crbug.com/844401#c2 it sounds like Extensions folks need to keep this around, exactly for this use case.
Hm, I think the content_shell_helper stuff is probably wrong and unused:

$ otool -L out/gn/Content\ Shell.app/Contents/Frameworks/Content\ Shell\ Helper.app/Contents/MacOS/Content\ Shell\ Helper 
out/gn/Content Shell.app/Contents/Frameworks/Content Shell Helper.app/Contents/MacOS/Content Shell Helper:
	/System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa (compatibility version 1.0.0, current version 22.0.0)
	/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1450.15.0)
	/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
	/System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 58286.31.2)
	/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration (compatibility version 1.0.0, current version 963.30.1)
	@rpath/Frameworks/Content Shell Framework.framework/Content Shell Framework (compatibility version 0.0.0, current version 0.0.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 400.9.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.0.0)


Compare to:

$ otool -L out/gn/App\ Shell.app/Contents/Frameworks/App\ Shell\ Helper.app/Contents/MacOS/App\ Shell\ Helper 
out/gn/App Shell.app/Contents/Frameworks/App Shell Helper.app/Contents/MacOS/App Shell Helper:
	@rpath/libcontent.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libbase.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libcc.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libcc_base.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libcc_paint.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libcc_debug.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libskia.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libcolor_space.dylib (compatibility version 0.0.0, current version 0.0.0)
        ...
Owner: thakis@chromium.org
Status: Started (was: Untriaged)
(looking now. why didn't anyone triage this though?)
Note to self:

DYLD_PRINT_RPATHS=1  out/gn/extensions_browsertests --gtest_filter=PrinterProviderApiTest.PrintJobFailed
Aha, half of this (the framework rpath) is a regression from https://chromium-review.googlesource.com/c/chromium/src/+/967155, the other half (helper stuff) from https://chromium-review.googlesource.com/c/chromium/src/+/958282
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 1 2018

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

commit b4a996805c069d26d5a63fcc79163a770310be12
Author: Nico Weber <thakis@chromium.org>
Date: Fri Jun 01 21:04:56 2018

mac: Make extension_browsertests work in component builds.

1. Make the helper not depend on //content/public/app:both, so that it
   doesn't have to be able to find all the .dylibs, this regressed in
   https://chromium-review.googlesource.com/c/chromium/src/+/958282

2. Fix the rpath on the framework so that the framework can find all the .dylibs,
   this regressed in https://chromium-review.googlesource.com/c/chromium/src/+/967155

Bug:  845455 
Change-Id: I591bb31660fb303cae90b528eed72eb7e27ee192
Reviewed-on: https://chromium-review.googlesource.com/1083093
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563793}
[modify] https://crrev.com/b4a996805c069d26d5a63fcc79163a770310be12/chrome/BUILD.gn
[modify] https://crrev.com/b4a996805c069d26d5a63fcc79163a770310be12/extensions/shell/BUILD.gn
[modify] https://crrev.com/b4a996805c069d26d5a63fcc79163a770310be12/extensions/shell/app/shell_main.cc

This issue is fixed. One of the tests in extensions_browsertests still fails on that bot, but that's just issue 844402 (turns out that one isn't asan-only).
Status: Fixed (was: Started)

Sign in to add a comment