Change PlatformEventController to take LocalFrame rather than Page.

Change PlatformEventController to take LocalFrame rather than Page.

This change is required to convert the Timer to a per-frame scheduler in
https://codereview.chromium.org/2648433003.
All callers are currently converting frame/document objects to page in any case, so there is little change needed.

BUG=624694
Review-Url: https://codereview.chromium.org/2645823002
Cr-Commit-Position: refs/heads/master@{#447492}
Committed: https://chromium.googlesource.com/chromium/src/+/b7d6f926eb0cf4e99dc6daac6e412381ae761a2e

[joelhockey, 2017-01-19 09:40:16]

joelhockey@chromium.org changed reviewers:
+ dcheng@chromium.org, haraken@chromium.org

[joelhockey, 2017-01-19 09:41:09]

Description was changed from

==========
Change PlatformEventController to take LocalFrame rather than Page.

This change is required to convert the Timer to a per-frame scheduler.
All callers are currently converting frame/document objects to page in any case,
so there is little change needed.

BUG=624694
==========

to

==========
Change PlatformEventController to take LocalFrame rather than Page.

This change is required to convert the Timer to a per-frame scheduler in
https://codereview.chromium.org/2648433003.
All callers are currently converting frame/document objects to page in any case,
so there is little change needed.

BUG=624694
==========

[joelhockey, 2017-01-19 09:41:26]

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

[commit-bot: I haz the power, 2017-01-19 09:41:55]

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

[haraken, 2017-01-19 09:48:40]

LGTM

[commit-bot: I haz the power, 2017-01-19 10:35:27]

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

[commit-bot: I haz the power, 2017-01-19 10:35:28]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[joelhockey, 2017-01-19 12:27:27]

The CQ bit was checked by joelhockey@chromium.org

[commit-bot: I haz the power, 2017-01-19 12:27:38]

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-01-19 13:19:40]

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

[commit-bot: I haz the power, 2017-01-19 13:19:41]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[joelhockey, 2017-01-19 13:22:24]

The CQ bit was checked by joelhockey@chromium.org

[commit-bot: I haz the power, 2017-01-19 13:22:32]

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-01-19 13:53:44]

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

[commit-bot: I haz the power, 2017-01-19 13:53:45]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[haraken, 2017-01-19 13:57:45]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2017-01-19 13:58:01]

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-01-19 14:20:09]

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

[commit-bot: I haz the power, 2017-01-19 14:20:11]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dcheng, 2017-01-20 07:18:17]

LGTM

https://codereview.chromium.org/2645823002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/PlatformEventController.cpp (right):

https://codereview.chromium.org/2645823002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/PlatformEventController.cpp:11:
PlatformEventController::PlatformEventController(LocalFrame* frame)
Nit: I think this can be passed as mutable ref. Blink convention is to pass by
mutable reference as a signal that something should never be null.

[joelhockey, 2017-01-20 09:35:23]

On 2017/01/19 at 14:20:11, commit-bot wrote:
> Try jobs failed on following builders:
>   mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

Tests are failing for this, and I just don't think I can get it to work to have
callers pass Frame rather than Page since in at least 1 case, Frame can be null.

So I'm going to abandon this change and go back to changing TaskRunnerHelper to
take a Frame input.
https://codereview.chromium.org/2645813002

This was causing failures at the code from 
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Dev...
when a document separates from its frame as per:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/dom/...

[joelhockey, 2017-02-01 07:12:56]

On 2017/01/20 at 09:35:23, joelhockey wrote:
> On 2017/01/19 at 14:20:11, commit-bot wrote:
> > Try jobs failed on following builders:
> >   mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
> 
> Tests are failing for this, and I just don't think I can get it to work to
have callers pass Frame rather than Page since in at least 1 case, Frame can be
null.
> 
> So I'm going to abandon this change and go back to changing TaskRunnerHelper
to take a Frame input.
> https://codereview.chromium.org/2645813002
> 
> This was causing failures at the code from 
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/Dev...
> when a document separates from its frame as per:
>
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/fast/dom/...

I have revived this change with an added null check on frame.
Note that now we pass page as null to PageVisibilityObserver when frame is null,
but I have checked and page was already null in this case.

[joelhockey, 2017-02-01 07:14:35]

The CQ bit was checked by joelhockey@chromium.org

[joelhockey, 2017-02-01 07:14:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2645823002/#ps20001
(title: "Change PlatformEventController to take LocalFrame rather than Page.")

[commit-bot: I haz the power, 2017-02-01 07:14: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-01 07:17:03]

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

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

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[joelhockey, 2017-02-01 07:17:29]

https://codereview.chromium.org/2645823002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/PlatformEventController.cpp (right):

https://codereview.chromium.org/2645823002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/PlatformEventController.cpp:11:
PlatformEventController::PlatformEventController(LocalFrame* frame)
On 2017/01/20 at 07:18:17, dcheng wrote:
> Nit: I think this can be passed as mutable ref. Blink convention is to pass by
mutable reference as a signal that something should never be null.

Daniel, I must confess I don't know a lot of C++.  When I've searched about
mutable references, I only see talk about the mutable keyword for variable
declarations.  I don't think that is what you are talking about.

So are you saying to change
PlatformEventController::PlatformEventController(LocalFrame* frame)
to
PlatformEventController::PlatformEventController(LocalFrame &frame)
or something else?

[joelhockey, 2017-02-01 07:25:36]

The CQ bit was checked by joelhockey@chromium.org

[joelhockey, 2017-02-01 07:25:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2645823002/#ps40001
(title: "Change PlatformEventController to take LocalFrame rather than Page.")

[commit-bot: I haz the power, 2017-02-01 07:25:48]

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-01 07:27:29]

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

[commit-bot: I haz the power, 2017-02-01 07:27:30]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[joelhockey, 2017-02-01 07:32:26]

The CQ bit was checked by joelhockey@chromium.org

[commit-bot: I haz the power, 2017-02-01 07:32:38]

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-01 07:34:18]

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

[commit-bot: I haz the power, 2017-02-01 07:34:19]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[dcheng, 2017-02-01 07:37:32]

On 2017/02/01 07:17:29, joelhockey wrote:
>
https://codereview.chromium.org/2645823002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/frame/PlatformEventController.cpp (right):
> 
>
https://codereview.chromium.org/2645823002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/frame/PlatformEventController.cpp:11:
> PlatformEventController::PlatformEventController(LocalFrame* frame)
> On 2017/01/20 at 07:18:17, dcheng wrote:
> > Nit: I think this can be passed as mutable ref. Blink convention is to pass
by
> mutable reference as a signal that something should never be null.
> 
> Daniel, I must confess I don't know a lot of C++.  When I've searched about
> mutable references, I only see talk about the mutable keyword for variable
> declarations.  I don't think that is what you are talking about.
> 
> So are you saying to change
> PlatformEventController::PlatformEventController(LocalFrame* frame)
> to
> PlatformEventController::PlatformEventController(LocalFrame &frame)
> or something else?

Yeah, this is what I meant. Sorry, I should have said non-const reference; I
think that would have been clearer.

[joelhockey, 2017-02-01 07:37:48]

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

[commit-bot: I haz the power, 2017-02-01 07:38: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-01 07:39:44]

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

[commit-bot: I haz the power, 2017-02-01 07:39:45]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[joelhockey, 2017-02-01 07:44:57]

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

[commit-bot: I haz the power, 2017-02-01 07:45:11]

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-01 07:46:58]

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

[commit-bot: I haz the power, 2017-02-01 07:46:59]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[joelhockey, 2017-02-01 10:47:53]

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

[commit-bot: I haz the power, 2017-02-01 10:48:02]

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

[joelhockey, 2017-02-01 11:39:40]

The CQ bit was unchecked by joelhockey@chromium.org

[joelhockey, 2017-02-01 11:39:45]

The CQ bit was checked by joelhockey@chromium.org

[joelhockey, 2017-02-01 11:39:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2645823002/#ps60001
(title: "Change PlatformEventController to take LocalFrame rather than Page.")

[commit-bot: I haz the power, 2017-02-01 11:39:53]

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-01 12:32:44]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485949185477880,
"parent_rev": "286225b8d7ea0ce861bf17be58df4af7a3a5e379", "commit_rev":
"0f14db42ae5e3c86b5652c3ea408328d43d12036"}

[commit-bot: I haz the power, 2017-02-01 12:36:00]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485949185477880,
"parent_rev": "13f00a7b6a700f280ec590e654cdbf223c510a45", "commit_rev":
"b7d6f926eb0cf4e99dc6daac6e412381ae761a2e"}

[commit-bot: I haz the power, 2017-02-01 12:36:32]

Description was changed from

==========
Change PlatformEventController to take LocalFrame rather than Page.

This change is required to convert the Timer to a per-frame scheduler in
https://codereview.chromium.org/2648433003.
All callers are currently converting frame/document objects to page in any case,
so there is little change needed.

BUG=624694
==========

to

==========
Change PlatformEventController to take LocalFrame rather than Page.

This change is required to convert the Timer to a per-frame scheduler in
https://codereview.chromium.org/2648433003.
All callers are currently converting frame/document objects to page in any case,
so there is little change needed.

BUG=624694

Review-Url: https://codereview.chromium.org/2645823002
Cr-Commit-Position: refs/heads/master@{#447492}
Committed:
https://chromium.googlesource.com/chromium/src/+/b7d6f926eb0cf4e99dc6daac6e41...
==========

[commit-bot: I haz the power, 2017-02-01 12:36:34]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b7d6f926eb0cf4e99dc6daac6e41...