Fix unload controller.

Fix unload controller.

There is a race when suddenly added beforeunload handler during window
closing can lead to trashing TabRestoreService.
(Browser::OnWindowClosing can be called several times with subsets of
previously opened tabs)

BUG=692879
Review-Url: https://codereview.chromium.org/2695233003
Cr-Commit-Position: refs/heads/master@{#454192}
Committed: https://chromium.googlesource.com/chromium/src/+/65a0b3cdd8b7605020143b3a5f089dc0878bdbb3

[Michael K. (Yandex Team), 2017-02-16 06:20:14]

Description was changed from

==========
Fix unload controller.

There is a race when suddenly added beforeunload handler during window
closing can lead to trashing TabRestoreService.
(Browser::OnWindowClosing can be called several times with subsets of
previously opened tabs)

BUG=692879
==========

to

==========
Fix unload controller.

There is a race when suddenly added beforeunload handler during window
closing can lead to trashing TabRestoreService.
(Browser::OnWindowClosing can be called several times with subsets of
previously opened tabs)

BUG=692879
==========

[Michael K. (Yandex Team), 2017-02-16 06:20:14]

mkolom@yandex-team.ru changed reviewers:
+ sky@chromium.org

[Michael K. (Yandex Team), 2017-02-16 06:27:45]

The CQ bit was checked by mkolom@yandex-team.ru to run a CQ dry run

[Michael K. (Yandex Team), 2017-02-16 06:28:20]

PTAL

[commit-bot: I haz the power, 2017-02-16 06:28:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 06:28:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-16 06:28:48]

Dry run: No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[sky, 2017-02-16 17:16:52]

sky@chromium.org changed reviewers:
+ creis@chromium.org

[sky, 2017-02-16 17:16:53]

+creis

This fix looks right to me, but one question.

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/lifetime...
File chrome/browser/lifetime/browser_close_manager_browsertest.cc (right):

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:124: service_ =
TabRestoreServiceFactory::GetForProfile(profile);
Use member initializer.

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:134: size_t
GetChangesCount() const { return changes_count_; }
GetChangesCount() -> changes_count()

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:137: void
TabRestoreServiceChanged(sessions::TabRestoreService*) override {
Prefix with where overrides come from, e.g.:
// sessions::TabRestoreServiceObserver:

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/ui/unloa...
File chrome/browser/ui/unload_controller.cc (right):

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/ui/unloa...
chrome/browser/ui/unload_controller.cc:304: if (HasCompletedUnloadProcessing()
&& !TabsNeedBeforeUnloadFired()) {
How come we don't always call TabsNeedBeforeUnloadFired?

[Michael K. (Yandex Team), 2017-02-17 09:43:04]

PTAL

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/lifetime...
File chrome/browser/lifetime/browser_close_manager_browsertest.cc (right):

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:124: service_ =
TabRestoreServiceFactory::GetForProfile(profile);
On 2017/02/16 17:16:53, sky wrote:
> Use member initializer.

Done.

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:134: size_t
GetChangesCount() const { return changes_count_; }
On 2017/02/16 17:16:53, sky wrote:
> GetChangesCount() -> changes_count()

Done.

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:137: void
TabRestoreServiceChanged(sessions::TabRestoreService*) override {
On 2017/02/16 17:16:53, sky wrote:
> Prefix with where overrides come from, e.g.:
> // sessions::TabRestoreServiceObserver:

Done.

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/ui/unloa...
File chrome/browser/ui/unload_controller.cc (right):

https://codereview.chromium.org/2695233003/diff/20001/chrome/browser/ui/unloa...
chrome/browser/ui/unload_controller.cc:304: if (HasCompletedUnloadProcessing()
&& !TabsNeedBeforeUnloadFired()) {
We do. We always call it.
But since we called it and then called all beforeunload handlers (and
HasCompletedUnloadProcessing() == true) situation could change: there could
appear new beforeunload handler(s) that we hadn't known about.
Thus when we think we are done we should check it again: call
TabsNeedBeforeUnloadFired and know whether situation has changed or not.

[sky, 2017-02-17 19:09:20]

LGTM

[Charlie Reis, 2017-02-17 21:05:51]

Thanks!  I'll defer to sky@ on the fix, since I don't know that code very well. 
One question about the case skipped by the test, but otherwise seems good.

https://codereview.chromium.org/2695233003/diff/40001/chrome/browser/lifetime...
File chrome/browser/lifetime/browser_close_manager_browsertest.cc (right):

https://codereview.chromium.org/2695233003/diff/40001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:749: // Currently
FastUnloadController is broken :(
What does this mean?  Does the bug still exist in some cases?  Please elaborate
in the comment.

Style nit: Please also put the TODO first with a bug number or username, such as
TODO(crbug.com/692879): ...
(Or file a new bug to track whatever this TODO is about.)

[Charlie Reis, 2017-02-17 21:05:59]

The CQ bit was checked by creis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 21:07:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 23:09:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 23:09:23]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[Michael K. (Yandex Team), 2017-02-20 06:25:21]

PTAL

https://codereview.chromium.org/2695233003/diff/40001/chrome/browser/lifetime...
File chrome/browser/lifetime/browser_close_manager_browsertest.cc (right):

https://codereview.chromium.org/2695233003/diff/40001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:749: // Currently
FastUnloadController is broken :(
Done.

There are two unload controllers: UnloadController and FastUnloadController
(which is still under experiment, see chrome://flags).
This CL fixes the first one - UnloadController.
But I was unsuccessful trying to fix this issue under FastUnloadController too:
sometimes I hit crbug.com/250305. Thus this test fails when running with
FastUnloadController enabled.
And currently I have no time to work on FastUnloadController, sorry.
Maybe it'll be changed in the near future..

If you LGTM on this, I'll add some notes about this test to crbug.com/250305.

[Charlie Reis, 2017-02-21 18:48:28]

LGTM.  I'll go ahead and put it in the CQ, since the earlier ChromeOS test
failures in PS3 didn't seem to be related.

https://codereview.chromium.org/2695233003/diff/40001/chrome/browser/lifetime...
File chrome/browser/lifetime/browser_close_manager_browsertest.cc (right):

https://codereview.chromium.org/2695233003/diff/40001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:749: // Currently
FastUnloadController is broken :(
On 2017/02/20 06:25:21, mkolom wrote:
> Done.
> 
> There are two unload controllers: UnloadController and FastUnloadController
> (which is still under experiment, see chrome://flags).
> This CL fixes the first one - UnloadController.
> But I was unsuccessful trying to fix this issue under FastUnloadController
too:
> sometimes I hit crbug.com/250305. Thus this test fails when running with
> FastUnloadController enabled.
> And currently I have no time to work on FastUnloadController, sorry.
> Maybe it'll be changed in the near future..
> 
> If you LGTM on this, I'll add some notes about this test to crbug.com/250305.

I see, thanks!  I didn't realize it was an experimental mode.

[Charlie Reis, 2017-02-21 18:48:34]

The CQ bit was checked by creis@chromium.org

[Charlie Reis, 2017-02-21 18:48:34]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2695233003/#ps60001
(title: "Fix TODO comment.")

[commit-bot: I haz the power, 2017-02-21 18:49:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-21 20:52:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-21 20:52:04]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[Charlie Reis, 2017-02-22 00:58:50]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2017-02-22 01:00:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 03:06:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 03:06:50]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[Michael K. (Yandex Team), 2017-02-22 05:17:17]

The CQ bit was checked by mkolom@yandex-team.ru

[commit-bot: I haz the power, 2017-02-22 05:18:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 09:56:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 09:56:19]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Michael K. (Yandex Team), 2017-02-22 10:09:18]

I took a look into logs and I haven't found any useful information what's
happened with 
linux_chromium_chromeos_rel_ng build.
Can anynone help me with that?

It seems I dont' have access to some urls where details are...

[sky, 2017-02-22 16:55:00]

The CQ bit was checked by sky@chromium.org

[sky, 2017-02-22 16:55:28]

On 2017/02/22 10:09:18, mkolom wrote:
> I took a look into logs and I haven't found any useful information what's
> happened with 
> linux_chromium_chromeos_rel_ng build.
> Can anynone help me with that?
> 
> It seems I dont' have access to some urls where details are...

Lets try one more time.

[commit-bot: I haz the power, 2017-02-22 16:55:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 18:14:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 18:14:58]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Charlie Reis, 2017-02-22 18:43:51]

On 2017/02/22 16:55:28, sky wrote:
> On 2017/02/22 10:09:18, mkolom wrote:
> > I took a look into logs and I haven't found any useful information what's
> > happened with 
> > linux_chromium_chromeos_rel_ng build.
> > Can anynone help me with that?
> > 
> > It seems I dont' have access to some urls where details are...
> 
> Lets try one more time.

Hmm, I'm not seeing these browser_tests fail on other try jobs, so I wonder if
this CL is somehow affecting them.  (Odd that they aren't failing on the other
platforms for you, though.)

Can you try running these locally to see if they pass or fail for you?
DevToolsExtensionTest.DevToolsExtensionSecurityPolicyGrants
DevToolsReattachAfterCrashTest.TestReattachAfterCrashOnNetwork
DevToolsReattachAfterCrashTest.TestReattachAfterCrashOnTimeline
DevToolsExtensionTest.DevToolsExtensionWithHttpIframe

Here's an example output from the bot:

DevToolsExtensionTest.DevToolsExtensionWithHttpIframe (run #1):
[ RUN      ] DevToolsExtensionTest.DevToolsExtensionWithHttpIframe
[28563:28563:0222/095314.638591:WARNING:chrome_browser_main_chromeos.cc(367)]
Running as stub user with profile dir: test-user
[28563:28563:0222/095314.687673:WARNING:audio_manager.cc(322)] Multiple
instances of AudioManager detected
[28563:28563:0222/095314.687706:WARNING:audio_manager.cc(281)] Multiple
instances of AudioManager detected
[28563:28563:0222/095314.708886:ERROR:logging_chrome.cc(175)] Unable to create
symlink /tmp/.org.chromium.Chromium.RTOkuF/dYifuTA/test-user/chrome_debug.log
pointing at
/tmp/.org.chromium.Chromium.RTOkuF/dYifuTA/test-user/chrome_debug_20170222-095314.log:
No such file or directory
Xlib:  extension "RANDR" missing on display ":99".
[28563:28563:0222/095314.930853:WARNING:signin_screen_policy_provider.cc(61)]
Denying load of Extension  : ahfgeienlihckogmohjhadlkjgocpleb / Web Store
because of 'signin_screen' is only allowed for packaged apps, but this is a
hosted app.
[28563:28563:0222/095314.930928:WARNING:signin_screen_policy_provider.cc(61)]
Denying load of Extension  : eemcgdkfndhakfknompkggombfjjjeno / Bookmark Manager
because of 'signin_screen' is only allowed for packaged apps, but this is a
extension.
[28563:28563:0222/095314.930974:WARNING:signin_screen_policy_provider.cc(61)]
Denying load of Extension  : mgndgikekgjfcpckkfioiadnlibdjbkf / Chromium because
of 'signin_screen' is only allowed for packaged apps, but this is a hosted app.
[28563:28563:0222/095314.931017:WARNING:signin_screen_policy_provider.cc(61)]
Denying load of Extension  : nkoccljplnhpfnfiajclkommnmllphnl / crosh_builtin
because of 'signin_screen' is only allowed for packaged apps, but this is a
extension.
[28563:28617:0222/095314.946517:WARNING:freezer_cgroup_process_manager.cc(61)]
Cgroup freezer does not exist or is not writable. Unable to freeze renderer
processes.
[28563:28563:0222/095315.076987:WARNING:signin_screen_policy_provider.cc(61)]
Denying load of Extension  : ahfgeienlihckogmohjhadlkjgocpleb / Web Store
because of 'signin_screen' is only allowed for packaged apps, but this is a
hosted app.
[28563:28563:0222/095315.077056:WARNING:signin_screen_policy_provider.cc(61)]
Denying load of Extension  : eemcgdkfndhakfknompkggombfjjjeno / Bookmark Manager
because of 'signin_screen' is only allowed for packaged apps, but this is a
extension.
[28563:28563:0222/095315.077097:WARNING:signin_screen_policy_provider.cc(61)]
Denying load of Extension  : mgndgikekgjfcpckkfioiadnlibdjbkf / Chromium because
of 'signin_screen' is only allowed for packaged apps, but this is a hosted app.
[28563:28563:0222/095315.077145:WARNING:signin_screen_policy_provider.cc(61)]
Denying load of Extension  : nkoccljplnhpfnfiajclkommnmllphnl / crosh_builtin
because of 'signin_screen' is only allowed for packaged apps, but this is a
extension.
HTTP server started on http://127.0.0.1:52531...
sending server_data: {"host": "127.0.0.1", "port": 52531} (36 bytes)
[28563:28628:0222/095315.559797:WARNING:simple_synchronous_entry.cc(1054)] Could
not open platform files for entry.
[28563:28563:0222/095317.146616:WARNING:tab_manager_delegate_chromeos.cc(89)]
Set OOM score error:
[1:1:0222/095317.410793:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for 'all'.
[1:1:0222/095317.411191:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for 'background-repeat-x'.
[1:1:0222/095317.411355:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for 'background-repeat-y'.
[1:1:0222/095317.412325:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for 'page'.
[1:1:0222/095317.412582:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for 'quotes'.
[1:1:0222/095317.412783:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for 'size'.
[1:1:0222/095317.413327:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for '-webkit-font-size-delta'.
[1:1:0222/095317.413604:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for '-webkit-mask-repeat-x'.
[1:1:0222/095317.413740:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for '-webkit-mask-repeat-y'.
[1:1:0222/095317.413894:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for '-webkit-perspective-origin-x'.
[1:1:0222/095317.414006:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for '-webkit-perspective-origin-y'.
[1:1:0222/095317.414185:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for '-webkit-transform-origin-x'.
[1:1:0222/095317.414291:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for '-webkit-transform-origin-y'.
[1:1:0222/095317.414416:ERROR:CSSComputedStyleDeclaration.cpp(207)] Blink does
not yet implement getComputedStyle for '-webkit-transform-origin-z'.
[28563:28563:0222/095317.727678:ERROR:CONSOLE(57)] "Uncaught TypeError: Cannot
read property 'addExtensions' of undefined", source:
chrome-devtools://devtools/bundled/devtools_compatibility.js (57)
[28563:28563:0222/095317.813414:INFO:CONSOLE(1)] "PASS", source:  (1)

[Michael K. (Yandex Team), 2017-03-01 07:46:01]

I see the problem: I recompiled with target_os = "chromeos" on my linux machine
and yes: some tests fail...

[Michael K. (Yandex Team), 2017-03-01 12:52:20]

The CQ bit was checked by mkolom@yandex-team.ru to run a CQ dry run

[commit-bot: I haz the power, 2017-03-01 12:52:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-01 13:36:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-01 13:36:06]

Dry run: This issue passed the CQ dry run.

[Michael K. (Yandex Team), 2017-03-01 13:44:02]

mkolom@yandex-team.ru changed reviewers:
+ dgozman@chromium.org

[Michael K. (Yandex Team), 2017-03-01 13:48:43]

dgozman is for devtools_window.cc


PTAL

[Michael K. (Yandex Team), 2017-03-01 13:48:52]

https://codereview.chromium.org/2695233003/diff/80001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/2695233003/diff/80001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:774: window->life_stage_ != kClosing;
On closing NeedsToInterceptBeforeUnload(..) shouldn't return true 'cause
InterceptPageBeforeUnload(..) isn't going to intercept beforeunload (See the
method InterceptPageBeforeUnload(..) above).

[dgozman, 2017-03-01 19:39:48]

devtools lgtm modulo comment

https://codereview.chromium.org/2695233003/diff/80001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/2695233003/diff/80001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:774: window->life_stage_ != kClosing;
On 2017/03/01 13:48:52, mkolom wrote:
> On closing NeedsToInterceptBeforeUnload(..) shouldn't return true 'cause
> InterceptPageBeforeUnload(..) isn't going to intercept beforeunload (See the
> method InterceptPageBeforeUnload(..) above).

Should we do |window->life_stage_ == kLoadCompleted| instead to match the method
above completely?

[Michael K. (Yandex Team), 2017-03-02 04:28:47]

https://codereview.chromium.org/2695233003/diff/80001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_window.cc (right):

https://codereview.chromium.org/2695233003/diff/80001/chrome/browser/devtools...
chrome/browser/devtools/devtools_window.cc:774: window->life_stage_ != kClosing;
On 2017/03/01 19:39:48, dgozman wrote:
> Should we do |window->life_stage_ == kLoadCompleted| instead to match the
method
> above completely?

I think you are right.
I'll make a new patchset.

[Michael K. (Yandex Team), 2017-03-02 04:29:38]

The CQ bit was checked by mkolom@yandex-team.ru

[Michael K. (Yandex Team), 2017-03-02 04:29:38]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org,
sky@chromium.org, dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2695233003/#ps100001
(title: "Fix review note.")

[commit-bot: I haz the power, 2017-03-02 04:30:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-02 06:11:48]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1488428978141000,
"parent_rev": "171f9cc03aa42e133e436364c843628df55752e7", "commit_rev":
"65a0b3cdd8b7605020143b3a5f089dc0878bdbb3"}

[commit-bot: I haz the power, 2017-03-02 06:12:53]

Description was changed from

==========
Fix unload controller.

There is a race when suddenly added beforeunload handler during window
closing can lead to trashing TabRestoreService.
(Browser::OnWindowClosing can be called several times with subsets of
previously opened tabs)

BUG=692879
==========

to

==========
Fix unload controller.

There is a race when suddenly added beforeunload handler during window
closing can lead to trashing TabRestoreService.
(Browser::OnWindowClosing can be called several times with subsets of
previously opened tabs)

BUG=692879

Review-Url: https://codereview.chromium.org/2695233003
Cr-Commit-Position: refs/heads/master@{#454192}
Committed:
https://chromium.googlesource.com/chromium/src/+/65a0b3cdd8b7605020143b3a5f08...
==========

[commit-bot: I haz the power, 2017-03-02 06:12:55]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/65a0b3cdd8b7605020143b3a5f08...