Add Resume logic of Purge+Suspend to TabManager.

Add Resume logic of Purge+Suspend to TabManager.

- To avoid breaking web, we need to resume a backgrounded renderer that
  was purged and suspended.
  (A backgrounded renderer is suspended for 120seconds).
  After resumed, the renderer is running for 10 seconds and is purged
  and suspended again.
- intent-to-implement-and-ship of background renderer's purge +
  suspend is
  https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
  https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJvI/edit?usp=sharing
- The feature is not enabled by default because
  purge-and-suspend-time is 0.

BUG=635419
Committed: https://crrev.com/8241a62d2ba05a3c1d55d0e087986b637658892b
Cr-Commit-Position: refs/heads/master@{#429222}

[tasak, 2016-10-28 03:35:43]

The CQ bit was checked by tasak@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 03:36:08]

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, 2016-10-28 04:59:25]

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

[commit-bot: I haz the power, 2016-10-28 04:59:26]

Dry run: This issue passed the CQ dry run.

[tasak, 2016-10-28 05:15:26]

Patchset #1 (id:1) has been deleted

[tasak, 2016-10-28 05:15:37]

The CQ bit was checked by tasak@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 05:15:49]

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

[tasak, 2016-10-28 05:33:22]

tasak@google.com changed reviewers:
+ chrisha@chromium.org, haraken@chromium.org

[tasak, 2016-10-28 05:33:22]

Would you review this CL?

I created this CL from https://codereview.chromium.org/2387603003

[commit-bot: I haz the power, 2016-10-28 06:02:56]

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

[commit-bot: I haz the power, 2016-10-28 06:02:57]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-10-28 06:25:48]

LGTM

https://codereview.chromium.org/2462513002/diff/20001/chrome/browser/memory/t...
File chrome/browser/memory/tab_manager.h (right):

https://codereview.chromium.org/2462513002/diff/20001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:277: content::WebContents*
GetWebContentsByContentsId(int64_t tab_contents_id);

GetWebContentsById ?

https://codereview.chromium.org/2462513002/diff/20001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:280: // next state.

Returns the next state of the purge and suspend.

https://codereview.chromium.org/2462513002/diff/20001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:281: PurgeAndSuspendState
NextPurgeAndSuspendState(

GetNextStateOfPurgeAndSuspend ?

[tasak, 2016-10-28 06:53:43]

Thank you for review.

https://codereview.chromium.org/2462513002/diff/20001/chrome/browser/memory/t...
File chrome/browser/memory/tab_manager.h (right):

https://codereview.chromium.org/2462513002/diff/20001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:277: content::WebContents*
GetWebContentsByContentsId(int64_t tab_contents_id);
On 2016/10/28 06:25:48, haraken wrote:
> 
> GetWebContentsById ?

Done.

https://codereview.chromium.org/2462513002/diff/20001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:280: // next state.
On 2016/10/28 06:25:48, haraken wrote:
> 
> Returns the next state of the purge and suspend.

Done.

https://codereview.chromium.org/2462513002/diff/20001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:281: PurgeAndSuspendState
NextPurgeAndSuspendState(
On 2016/10/28 06:25:48, haraken wrote:
> 
> GetNextStateOfPurgeAndSuspend ?

Done.

[tasak, 2016-11-01 03:52:58]

chrisha, would you review this CL?

Does this CL seem ok?

[chrisha, 2016-11-01 21:22:56]

lgtm with nits

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
File chrome/browser/memory/tab_manager.cc (right):

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.cc:430: TabStripModel* model;
 = nullptr

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.cc:713: current_time -
Move this to the previous line

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
File chrome/browser/memory/tab_manager.h (right):

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:276: // Returns WebContents whose contents
id matches the given tab-contents-id.
|tab_contents_id|

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:279: // Returns the next state of the purge
and suspend.
This comment isn't very illuminating. What is |purge_and_suspend_threshold|?

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:282: const base::TimeTicks& current_time,
TimeTicks are overwhelmingly passed by value in Chrome code.

[tasak, 2016-11-02 04:17:28]

The CQ bit was checked by tasak@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-02 04:17:45]

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

[tasak, 2016-11-02 04:19:47]

Thank you for review.

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
File chrome/browser/memory/tab_manager.cc (right):

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.cc:430: TabStripModel* model;
On 2016/11/01 21:22:56, chrisha (slow) wrote:
>  = nullptr

Done.

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.cc:713: current_time -
On 2016/11/01 21:22:56, chrisha (slow) wrote:
> Move this to the previous line

Done.

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
File chrome/browser/memory/tab_manager.h (right):

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:276: // Returns WebContents whose contents
id matches the given tab-contents-id.
On 2016/11/01 21:22:56, chrisha (slow) wrote:
> |tab_contents_id|

Done.

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:279: // Returns the next state of the purge
and suspend.
On 2016/11/01 21:22:56, chrisha (slow) wrote:
> This comment isn't very illuminating. What is |purge_and_suspend_threshold|?

I renamed purge_and_suspend_threshold. Now it is time_to_first_suspension.

https://codereview.chromium.org/2462513002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.h:282: const base::TimeTicks& current_time,
On 2016/11/01 21:22:56, chrisha (slow) wrote:
> TimeTicks are overwhelmingly passed by value in Chrome code.

Done.

[commit-bot: I haz the power, 2016-11-02 05:05:42]

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

[commit-bot: I haz the power, 2016-11-02 05:05:42]

Dry run: This issue passed the CQ dry run.

[tasak, 2016-11-02 06:28:03]

The CQ bit was checked by tasak@google.com

[tasak, 2016-11-02 06:28:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, chrisha@chromium.org
Link to the patchset: https://codereview.chromium.org/2462513002/#ps60001
(title: "Fixed")

[commit-bot: I haz the power, 2016-11-02 06:28:19]

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, 2016-11-02 06:32:38]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-11-02 06:34:57]

Description was changed from

==========
Add Resume logic of Purge+Suspend to TabManager.

- To avoid breaking web, we need to resume a backgrounded renderer that
  was purged and suspended.
  (A backgrounded renderer is suspended for 120seconds).
  After resumed, the renderer is running for 10 seconds and is purged
  and suspended again.
- intent-to-implement-and-ship of background renderer's purge +
  suspend is
  https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
 
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- The feature is not enabled by default because
  purge-and-suspend-time is 0.

BUG=635419
==========

to

==========
Add Resume logic of Purge+Suspend to TabManager.

- To avoid breaking web, we need to resume a backgrounded renderer that
  was purged and suspended.
  (A backgrounded renderer is suspended for 120seconds).
  After resumed, the renderer is running for 10 seconds and is purged
  and suspended again.
- intent-to-implement-and-ship of background renderer's purge +
  suspend is
  https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
 
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- The feature is not enabled by default because
  purge-and-suspend-time is 0.

BUG=635419

Committed: https://crrev.com/8241a62d2ba05a3c1d55d0e087986b637658892b
Cr-Commit-Position: refs/heads/master@{#429222}
==========

[commit-bot: I haz the power, 2016-11-02 06:35:00]

Patchset 3 (id:??) landed as
https://crrev.com/8241a62d2ba05a3c1d55d0e087986b637658892b
Cr-Commit-Position: refs/heads/master@{#429222}

[kjellander_chromium, 2016-11-02 08:12:27]

A revert of this CL (patchset #3 id:60001) has been created in
https://codereview.chromium.org/2470073002/ by kjellander@chromium.org.

The reason for reverting is: Reliably breaks Linux ChromiumOS Tests (1):
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...
and Linux ChromiumOS Tests (dbg)(1):
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%....

[tasak, 2016-11-02 09:06:49]

On 2016/11/02 08:12:27, kjellander_chromium wrote:
> A revert of this CL (patchset #3 id:60001) has been created in
> https://codereview.chromium.org/2470073002/ by mailto:kjellander@chromium.org.
> 
> The reason for reverting is: Reliably breaks Linux ChromiumOS Tests (1):
>
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...
> and Linux ChromiumOS Tests (dbg)(1):
>
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%....

Before applying the revert patch, Linux%20ChromiumOS%20Tests%20%281%29 became
green:
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...

So I would like to ask the reason why my patch causes the issue.
As far as I understand, my patch is disabled by default by the switch:
--purge-and-suspend-time.