Introduce WebPageImportanceSignals to judge state preservation needs

Introduce WebPageImportanceSignals to judge state preservation needs

This CL introduces WebPageImportanceSignals, which is measured per Page.
We plan to propagate this signal to content/ so that Chromium can judge
the importance of the tab. Chromium can then use this signal to choose which
actions to take against the tab when we are approaching system resource limit.
For example:
- Simply discard the tab if user hadn't done anything with the tab.
- If he/she entered something on a form, we better preserve its state, etc.

To start with, we record if user has interacted with a HTML form on the page.

BUG=520838
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200630

[kouhei (in TOK), 2015-08-14 05:43:47]

kouhei@chromium.org changed reviewers:
+ haraken@chromium.org, keishi@chromium.org, tzik@chromium.org

[kouhei (in TOK), 2015-08-14 05:43:48]

PTAL

[kouhei (in TOK), 2015-08-14 05:45:48]

kouhei@chromium.org changed reviewers:
+ georgesak@chromium.org

[kouhei (in TOK), 2015-08-14 05:45:49]

+georgesak: FYI. We want to plumb this to tab_stats in future.

[tzik, 2015-08-14 07:42:57]

lgtm

[keishi, 2015-08-14 07:45:45]

LGTM

[kouhei (in TOK), 2015-08-14 07:46:03]

The CQ bit was checked by kouhei@chromium.org

[kouhei (in TOK), 2015-08-14 07:46:07]

The CQ bit was unchecked by kouhei@chromium.org

[kouhei (in TOK), 2015-08-14 07:46:12]

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

[commit-bot: I haz the power, 2015-08-14 07:46:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1296533003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1296533003/20001

[commit-bot: I haz the power, 2015-08-14 09:18:39]

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

[commit-bot: I haz the power, 2015-08-14 09:18:39]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[haraken, 2015-08-14 14:42:50]

LGTM

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
File Source/core/frame/PageImportanceSignals.cpp (right):

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
Source/core/frame/PageImportanceSignals.cpp:19: m_hadFormInteraction = false;

m_didObserveFormInteraction?

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
Source/core/frame/PageImportanceSignals.cpp:22: void
PageImportanceSignals::setHadFormInteraction()

didObserveFormInteraction?

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
File Source/core/frame/PageImportanceSignals.h (right):

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
Source/core/frame/PageImportanceSignals.h:10: class PageImportanceSignals {

Would you add a comment about what it is?

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
Source/core/frame/PageImportanceSignals.h:10: class PageImportanceSignals {

Add DISALLOW_ALLOCATION().

https://codereview.chromium.org/1296533003/diff/20001/Source/core/page/Page.h
File Source/core/page/Page.h (right):

https://codereview.chromium.org/1296533003/diff/20001/Source/core/page/Page.h...
Source/core/page/Page.h:152: PageImportanceSignals& importanceSignals() { return
m_importanceSignals; }

Add const.

[kouhei (in TOK), 2015-08-17 03:31:31]

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
File Source/core/frame/PageImportanceSignals.cpp (right):

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
Source/core/frame/PageImportanceSignals.cpp:19: m_hadFormInteraction = false;
On 2015/08/14 14:42:50, haraken wrote:
> 
> m_didObserveFormInteraction?

discussed offline. keeping as is.

https://codereview.chromium.org/1296533003/diff/20001/Source/core/page/Page.h
File Source/core/page/Page.h (right):

https://codereview.chromium.org/1296533003/diff/20001/Source/core/page/Page.h...
Source/core/page/Page.h:152: PageImportanceSignals& importanceSignals() { return
m_importanceSignals; }
On 2015/08/14 14:42:50, haraken wrote:
> 
> Add const.

importanceSignals needs to be mutable here, so that we can update signals from
various core/ code.
While importanceSignals() is syntactically valid, it is confusing.

[kouhei (in TOK), 2015-08-17 03:44:36]

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
File Source/core/frame/PageImportanceSignals.h (right):

https://codereview.chromium.org/1296533003/diff/20001/Source/core/frame/PageI...
Source/core/frame/PageImportanceSignals.h:10: class PageImportanceSignals {
On 2015/08/14 14:42:50, haraken wrote:
> 
> Would you add a comment about what it is?

Done.

[kouhei (in TOK), 2015-08-17 03:44:41]

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

[commit-bot: I haz the power, 2015-08-17 03:44:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1296533003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1296533003/40001

[commit-bot: I haz the power, 2015-08-17 03:53:49]

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

[commit-bot: I haz the power, 2015-08-17 03:53:50]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kouhei (in TOK), 2015-08-17 04:11:01]

The CQ bit was checked by kouhei@chromium.org

[kouhei (in TOK), 2015-08-17 04:11:02]

The patchset sent to the CQ was uploaded after l-g-t-m from keishi@chromium.org,
haraken@chromium.org, tzik@chromium.org
Link to the patchset: https://codereview.chromium.org/1296533003/#ps50009
(title: "add missing includes")

[commit-bot: I haz the power, 2015-08-17 04:11:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1296533003/50009
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1296533003/50009

[tkent, 2015-08-17 04:15:20]

I think this should not be in 'core'.  Don't we have enough hooks with which we
can implement PageImportanceSignals in web/ ?

[kouhei (in TOK), 2015-08-17 04:15:46]

The CQ bit was unchecked by kouhei@chromium.org

[tkent, 2015-08-17 04:17:56]

tkent@chromium.org changed reviewers:
+ tkent@chromium.org

[tkent, 2015-08-17 04:17:56]

https://codereview.chromium.org/1296533003/diff/50009/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormControlElement.cpp (right):

https://codereview.chromium.org/1296533003/diff/50009/Source/core/html/HTMLFo...
Source/core/html/HTMLFormControlElement.cpp:322: void
HTMLFormControlElement::dispatchChangeEvent()
Input events are more appropriate than change events.

[kouhei (in TOK), 2015-08-17 05:14:27]

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

[commit-bot: I haz the power, 2015-08-17 05:14:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1296533003/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1296533003/70001

[kouhei (in TOK), 2015-08-17 05:17:10]

tkent-san. PTAL

https://codereview.chromium.org/1296533003/diff/50009/Source/core/html/HTMLFo...
File Source/core/html/HTMLFormControlElement.cpp (right):

https://codereview.chromium.org/1296533003/diff/50009/Source/core/html/HTMLFo...
Source/core/html/HTMLFormControlElement.cpp:322: void
HTMLFormControlElement::dispatchChangeEvent()
On 2015/08/17 04:17:56, tkent wrote:
> Input events are more appropriate than change events.

Ack. For this CL, we will reuse ChromeClientImpl::didChangeValueInTextField
hook. We will add a similar hook to catch InputEvent in ChromeChlientImpl.

[kouhei (in TOK), 2015-08-17 05:19:18]

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

[commit-bot: I haz the power, 2015-08-17 05:19:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1296533003/90001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1296533003/90001

[tkent, 2015-08-17 05:26:29]

lgtm

https://codereview.chromium.org/1296533003/diff/90001/public/web/WebPageImpor...
File public/web/WebPageImportanceSignals.h (right):

https://codereview.chromium.org/1296533003/diff/90001/public/web/WebPageImpor...
public/web/WebPageImportanceSignals.h:8: #include "../platform/WebCommon.h"
Please do not use relative include path.
#include "public/platform/WebCommon.h"

https://codereview.chromium.org/1296533003/diff/90001/public/web/WebPageImpor...
public/web/WebPageImportanceSignals.h:28: bool m_hadFormInteraction:1;
We usually put spaces before&after ':' for bitfield declarations.

[kouhei (in TOK), 2015-08-17 05:28:35]

The CQ bit was checked by kouhei@chromium.org

[kouhei (in TOK), 2015-08-17 05:28:36]

The patchset sent to the CQ was uploaded after l-g-t-m from keishi@chromium.org,
haraken@chromium.org, tzik@chromium.org, tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1296533003/#ps110001
(title: "nits")

[kouhei (in TOK), 2015-08-17 05:28:45]

Thanks!

https://codereview.chromium.org/1296533003/diff/90001/public/web/WebPageImpor...
File public/web/WebPageImportanceSignals.h (right):

https://codereview.chromium.org/1296533003/diff/90001/public/web/WebPageImpor...
public/web/WebPageImportanceSignals.h:8: #include "../platform/WebCommon.h"
On 2015/08/17 05:26:29, tkent wrote:
> Please do not use relative include path.
> #include "public/platform/WebCommon.h"

Done.

https://codereview.chromium.org/1296533003/diff/90001/public/web/WebPageImpor...
public/web/WebPageImportanceSignals.h:28: bool m_hadFormInteraction:1;
On 2015/08/17 05:26:29, tkent wrote:
> We usually put spaces before&after ':' for bitfield declarations.
> 

Done.

[commit-bot: I haz the power, 2015-08-17 05:28:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1296533003/110001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1296533003/110001

[commit-bot: I haz the power, 2015-08-17 06:39:07]

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

[commit-bot: I haz the power, 2015-08-17 06:39:08]

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

[kouhei (in TOK), 2015-08-17 06:39:28]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2015-08-17 06:39:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1296533003/110001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1296533003/110001

[commit-bot: I haz the power, 2015-08-17 07:17:10]

Committed patchset #7 (id:110001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200630