Issue metadata
Sign in to add a comment
|
Opening links from external applications doesn't work |
||||||||||||||||||||||
Issue descriptionChrome 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".
,
Jul 5 2017
,
Jul 5 2017
Thanks! I found more necessary steps, see if you can reproduce it now.
,
Jul 6 2017
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.
,
Jul 6 2017
Suspecting 7afbc36eeee75f34c8ad3aa2f90cb6fc1f345463. eugenebng@, thoughts?
,
Jul 6 2017
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.
,
Jul 6 2017
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.
,
Jul 6 2017
Yes, I can reproduce it by opening an HTML file — screen recording attached.
,
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?
,
Jul 6 2017
Issue 739566 has been merged into this issue.
,
Jul 6 2017
,
Jul 7 2017
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.
,
Jul 7 2017
,
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.
,
Jul 11 2017
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 .
,
Jul 11 2017
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.
,
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.
,
Jul 11 2017
Ok, I will take and do the revert.
,
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
,
Jul 12 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by jmukthavaram@chromium.org
, Jul 5 2017Labels: Needs-Feedback
1.2 MB
1.2 MB View Download