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

Issue 739200 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Opening links from external applications doesn't work

Project Member Reported by sdy@chromium.org, Jul 4 2017

Issue description

Chrome Version: 61.0.3148.0 (good: 60.0.3112.50)
OS: macOS 10.12.5

What steps will reproduce the problem?
(1) (Optional) in System Preferences -> General, change your default browser to Chrome Canary.
(2) Launch Chrome
(3) In Chrome, change your "On startup" setting to "Continue where you left off".
(4) Close all tabs/windows using either cmd+W or File > Close Tab. Closing the whole window or closing tabs with the 'x' button will not trigger the issue.
(5) Click a link in an external application, or run "open https://google.com" in a terminal. (When testing, you can use "open -a /path/to/Chrome.app https://google.com" instead of changing your default browser).

What is the expected result?
The link opens in a new window.

What happens instead?
A new window opens with no tabs. The omnibox says "about:blank".
 
crbug_739200_demo.mp4
1.8 MB View Download
Screen Shot 2017-07-04 at 9.13.58 AM.png
76.7 KB View Download
Cc: jmukthavaram@chromium.org
Labels: Needs-Feedback
Unable to reproduce the issue on Mac 10.12.5 using chrome latest Canary#61.0.3148.0 as per below steps:

Procedure 1:
----------
1. Closed all the windows
2. Set in System Preferences -> General, changed default browser to Chrome Canary
3. Open terminal
4. Type 'open https://google.com' and hit enter
5. Observed new google window opened without any issue.

Procedure 2:
-----------
1. Closed all the windows
2. General settings,default browser is not set to Chrome Canary
3. Open terminal
4. Type 'open -a canarypath  https://google.com and hit enter
5. Observed new google window opened without any issue.

Please find the attached screencast for reference and let us know your observations on the same.
Thanks..!! 

739200.mp4
1.2 MB View Download

Comment 2 by sdy@chromium.org, Jul 5 2017

Description: Show this description

Comment 3 by sdy@chromium.org, Jul 5 2017

Labels: -Needs-Feedback
Thanks! I found more necessary steps, see if you can reproduce it now.
Labels: -Needs-Bisect
Sdy@,
Thanks for the demo.
Able to reproduce the issue as per demo on Mac 10.12.5 using chrome latest Canary-61.0.3150.0.

Manual bisect info:
------------------
Good-61.0.3131.0-Revision-479564
Bad-61.0.3132.0-Revision-479900

Unable to provide bisect for this issue as it is reproduced through terminal.Hence marking it as Untriaged to get more inputs from dev for further investigation.

Thanks.


Comment 5 by sdy@chromium.org, Jul 6 2017

Cc: eugene...@yandex-team.ru a...@chromium.org
Status: Available (was: Untriaged)
Suspecting 7afbc36eeee75f34c8ad3aa2f90cb6fc1f345463. eugenebng@, thoughts?

Comment 6 by sdy@chromium.org, Jul 6 2017

Owner: sky@chromium.org
Status: Assigned (was: Available)
sky@, I'm assigning you so that this issue has an owner, but feel free to kick it to someone else. It looks like a revert was started from issue 735958 but a different fix was landed instead.
sdy@, sky@ it would be very helpful to know if this bug reproduces with opening html file from Finder (right-click on html file in Finder (or on Desktop), open with, choose Chromium app being tested) - a replacement URL-opening step for this bug.
In my CL there is a test that simulates opens link in this case via Apple Event.

If it turns out that opening URL passed via Apple event in test (hope nobody disabled it?) is okay and doing repro steps of this bug , but with opening html file from Finder/Desktop is okay too,
then it is unfortunately quite likely that I've overlooked the case described in this bug and broken it in mu CL mentioned. 

Comment 8 by sdy@chromium.org, Jul 6 2017

Yes, I can reproduce it by opening an HTML file — screen recording attached.
crbug_739200_open_html.mp4
768 KB View Download

Comment 9 by sdy@chromium.org, Jul 6 2017

I bet it has to do with "continue where I left off" plus explicitly closing the last tab. Maybe your test sets up a different kind of environment?

Comment 10 by sdy@chromium.org, Jul 6 2017

Cc: sky@chromium.org sdy@chromium.org dominicc@chromium.org
 Issue 739566  has been merged into this issue.

Comment 11 by sdy@chromium.org, Jul 6 2017

Labels: -ReleaseBlock-Stable ReleaseBlock-Beta
sdy@, you are right, this is because of explicitly closing the last tab. Closing tab, but not window, leaves application in a certain state, that is not handled correctly by URL opening logic from my patch 7afbc36eeee75f34c8ad3aa2f90cb6fc1f345463. 
That state is somehow different from the application state when window (but not tab) is closed - after-the-window-is-closed state is handled correctly in code from my CL.
We know that pref.type is SessionStartupPref::LAST, so the points regarding the state are:
- is there active session restorers (see fix for https://bugs.chromium.org/p/chromium/issues/detail?id=735958),
- is session being restored,
- is there active browser - GetLastActiveBrowser(),
- if there is active browser, what's its state: visible or not, has any tab in tab strip or not.
Unfortunately, I can't help with debugging myself right now.
Cc: ligim...@chromium.org

Comment 14 by ajha@chromium.org, Jul 10 2017

Just to update, M-61 gets branched in 10 days time and would be good to have all the Beta blockers resolved before branch point. Please plan and land the fix accordingly.

Comment 15 by sky@chromium.org, Jul 11 2017

Owner: a...@chromium.org
I'm not a good owner for this as it's Mac specific. Given we have had other problems with https://crrev.com/7afbc36eeee75f34c8ad3aa2f90cb6fc1f345463 I'm inclined to revert it. Avi, are you interested in trying to fix this? If not, I'll try to revert https://crrev.com/7afbc36eeee75f34c8ad3aa2f90cb6fc1f345463 .
A friendly reminder that M61 branch is coming soon on 07/20! Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix ASAP to trunk. This way we branch M61 from a high quality trunk. Thank you.

Comment 17 by a...@chromium.org, Jul 11 2017

This is what worried me about that patch, because this is not a very well-tested area of the code. At this point, since we keep coming up with issue, we should revert and try again for m62.

Comment 18 by sky@chromium.org, Jul 11 2017

Owner: sky@chromium.org
Ok, I will take and do the revert.
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 12 2017

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

commit 1f106b584b528fc2d2e84660da8c9ae99eb4178a
Author: Scott Violet <sky@chromium.org>
Date: Wed Jul 12 15:49:58 2017

Revert: Fix for URL opening code

Reverting as has caused a couple of regressions. Latest is opening
links from external applications doesn't work.

BUG=708873, 739200 
TEST=verify 739200 is fixed, 708873 will be broke again

Change-Id: I5275694888bcead75deb44450e7e6e1192c17adf
Reviewed-on: https://chromium-review.googlesource.com/567562
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485974}
[modify] https://crrev.com/1f106b584b528fc2d2e84660da8c9ae99eb4178a/chrome/browser/app_controller_mac.mm
[modify] https://crrev.com/1f106b584b528fc2d2e84660da8c9ae99eb4178a/chrome/browser/app_controller_mac_browsertest.mm
[modify] https://crrev.com/1f106b584b528fc2d2e84660da8c9ae99eb4178a/chrome/browser/lifetime/browser_close_manager_browsertest.cc
[modify] https://crrev.com/1f106b584b528fc2d2e84660da8c9ae99eb4178a/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
[modify] https://crrev.com/1f106b584b528fc2d2e84660da8c9ae99eb4178a/chrome/browser/sessions/session_restore.cc
[modify] https://crrev.com/1f106b584b528fc2d2e84660da8c9ae99eb4178a/chrome/browser/sessions/session_restore.h
[modify] https://crrev.com/1f106b584b528fc2d2e84660da8c9ae99eb4178a/chrome/browser/sessions/session_restore_browsertest.cc
[delete] https://crrev.com/b789d5d5b73cdf5d7afd99eaa19dac801811d296/content/public/test/repeated_notification_observer.cc
[delete] https://crrev.com/b789d5d5b73cdf5d7afd99eaa19dac801811d296/content/public/test/repeated_notification_observer.h
[modify] https://crrev.com/1f106b584b528fc2d2e84660da8c9ae99eb4178a/content/test/BUILD.gn

Comment 20 by sky@chromium.org, Jul 12 2017

Status: Fixed (was: Assigned)

Sign in to add a comment