I had this happen to me today also, on Windows, albeit in a slightly different form. I needed to restart my machine in order to apply updates. I closed Chrome (although I didn't check to see if it had closed cleanly) and after rebooting found that Chrome had restored my tabs (as expected) but also said "Chrome didn't shut down correctly.".

As an experiment I clicked the infobar and I got all of my tabs reopened again.

In my case this happened on Version 56.0.2924.87, so maybe this is a totally different bug, but there's enough overlap that I'm adding this information here.

@Reporter: Investigating. I don't think the culprit CL you've identified is correct, though. The behavior of kRestoreLastSession overriding crash behavior already existed and that CL just expresses it using a bitmask. This likely happens in M57 as well.

c#1, I don't think this is related. Startup code changed pretty substantially between M56 and M57.

You're right - this bug appears to have been introduced in https://codereview.chromium.org/2457653003, which landed in M56 and was merged back to M55.

N.b. That codepath was disabled by default until M57, which is why it's surfacing now.

A friendly reminder that M58 Stable launch is coming soon! Your bug is labelled as Stable ReleaseBlock, please make sure to land the fix, verified in trunk and get it merged into the release branch ASAP.

On investigation, it's worth noting that this only seems to trigger if the "Reopen windows when logging in" box was checked on the last clean shutdown.

(i.e., having Chrome as a login item is not sufficient.)

OK, after further testing, it looks like this behavior was not introduced when I rewrote the launch portions of StartupBrowserCreatorImpl--that rewrite merely replicates behavior that has existed for much longer. I was able to reproduce this on M56 (which does not use my refactor).

Since the culprit logic in chrome_browser_main_mac was added in 2014 [1] and the culprit logic in StartupBrowserCreatorImpl dates back to 2012 [2] or possibly earlier, I'm gonna go ahead and say this is a very long-standing bug. Removing ReleaseBlock-Stable, but leaving as P1.

I'll get to fixing this in the near-term, but it's not completely obvious yet what the desired behavior is. It's an open question what behaviors the --restore-last-session flag should/shouldn't override, and whether it's appropriate for chrome_browser_main_mac to be invoking session restore via flag or if there's another mechanism we could introduce.

[1] https://chromium.googlesource.com/chromium/src/+/769ddfe1ab93eabec7934ebaeb8228193723e83c%5E%21/chrome/browser/chrome_browser_main_mac.mm

[2] https://chromium.googlesource.com/chromium/src/+/cc2806854ca9c81d8175a2c318f5a6814a884792%5E%21/chrome/browser/ui/startup/startup_browser_creator_impl.cc

+rpop@

This bug is a disaster for anyone who encounters it. We need to target a milestone for getting it fixed (as in make it a release blocker for that M) rather than let it float as a P1 (with the 700+ other Pri=1 OS=Mac bugs).

+Chris, can we find someone to own this? 

IMO the correct behavior is for the session restore to NOT occur, and for the window to be presented. This has always been the intended behavior because the unclean shutdown could have been caused by a crash related to one of the pages being loaded, and this prevents an automatic "crash loop".

As a separate issue we want to annotate startups after an unclean shutdowns with metadata about the unclean shutdown. If the shutdown can be inferred to be due to the machine going down (battery died, loss of power, etc) than we think it would be appropriate to auto session-restore in this case. (On Windows we can infer this from the system logs, some investigation required for other platforms.)  

"for the window to be presented" should read "for the infobar to be presented"

There's also another separate bug here: if session restore is forced on via a flag, then the infobar should not be presented.

Had some more time to dig here:

The semantics of "restore-last-session" are unclear.

1. In startup_browser_creator_impl.cc:995 we allow the command-line flag to force enable session restore, even if a crash has occurred.
2. On Mac we manually add the flag in chrome_browser_main_mac.mm:78 if 'WasLaunchedAsLoginItemRestoreState'.
3. On Win we manually add the flag to the command-line that the OS "Restart Manager" calls.

2 and 3 are slightly at odds with 1. The OS "continue where I left off" logic will forcibly cause a session restore to occur, even if the previous shutdown cause was a crash.

There are 2 ways to proceed, IMO:

1. Change the semantics of the flag such that it only overrides the prefs setting, and *not* the last-shutdown-type=CRASH logic.
2. Add a new flag for use by restart managers, which is distinct from "restore-last-session". Something like "restore-last-session-unless-crash".

There is still the separate issue of the session restore occurring automatically, and also immediately being offered to the user post-crash (resulting in double session restore).

Any strong opinions out there?

I like your option 1.

In the interest of simplicity and consistency I've gone with option 1. 

Getting this to reproduce on both Mac and Windows is easy:

(1) Open the browser.
(2) Create several tabs.
(3) Forcibly kill the browser process to cause a crash to be detected on next restart.
(4) Restart the browser with the "--restore-last-session" flag.

Expected behaviour:

- Session restore doesn't automatically occur. A New Tab Page is opened, and a bubble is opened offering to restore the session post-crash. Clicking "restore" causes the new tab page to close and the session to be restored.

Actual behaviour:

- Session restore automatically occurs. A New Tab Page is additionally created and focus given to it. A bubble is opened offering to restore the session post-crash. Clicking "restores" causes the session to be restored as second time, and the new tab remains as well.

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

commit 92f8ca3fa43fbe22f303afc4c464924dccd1ccbe
Author: chrisha <chrisha@chromium.org>
Date: Wed Apr 19 17:54:47 2017

Fix double session-restore bug.

Currently on a post-crash startup with the --restore-last-session flag specified an automatic session restore occurs, and an infobar offering an additional session restore is created. Clicking "restore" on the infobar results in the session being restored twice.

This CL removes the logic that makes the flag automatically restore the session post-crash. This diminishes the power of the flag somewhat, but allows it to continue working in support of OS-specific "continue where you left of" functionality on Windows and OS X.

BUG= 707998 

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

[modify] https://crrev.com/92f8ca3fa43fbe22f303afc4c464924dccd1ccbe/chrome/browser/ui/startup/startup_browser_creator_impl.cc
[modify] https://crrev.com/92f8ca3fa43fbe22f303afc4c464924dccd1ccbe/chrome/browser/ui/startup/startup_browser_creator_impl.h
[modify] https://crrev.com/92f8ca3fa43fbe22f303afc4c464924dccd1ccbe/chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc
[modify] https://crrev.com/92f8ca3fa43fbe22f303afc4c464924dccd1ccbe/chrome/common/chrome_switches.cc

Issue 722157 has been merged into this issue.