Add Page.stopLoading to devtools and --timeout switch to headless_shell

Add Page.stopLoading to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable timeframe so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.stopLoading devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
R=dgozman@chromium.org, skyostil@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/6f5c58a6767e018775b02060009a432ea81a74db

[alex clarke (OOO till 29th), 2016-11-23 18:03:56]

alexclarke@chromium.org changed reviewers:
+ skyostil@chromium.org

[Sami, 2016-11-23 18:11:32]

https://codereview.chromium.org/2525903003/diff/1/headless/app/headless_shell.cc
File headless/app/headless_shell.cc (right):

https://codereview.chromium.org/2525903003/diff/1/headless/app/headless_shell...
headless/app/headless_shell.cc:176: base::Bind(&HeadlessShell::FetchTimeout,
base::Unretained(this)),
Should we use a weak pointer here? Nothing seems to prevent this from firing
after the shell is gone.

https://codereview.chromium.org/2525903003/diff/1/headless/lib/browser/headle...
File headless/lib/browser/headless_web_contents_impl.h (right):

https://codereview.chromium.org/2525903003/diff/1/headless/lib/browser/headle...
headless/lib/browser/headless_web_contents_impl.h:77: void Stop() override;
We'll need this in devtools too, right? Should we just add it there from the
beginning? (Sorry but this is the question I ask myself for every API we add
here :)

[alex clarke (OOO till 29th), 2016-11-24 12:05:04]

PTAL

https://codereview.chromium.org/2525903003/diff/1/headless/app/headless_shell.cc
File headless/app/headless_shell.cc (right):

https://codereview.chromium.org/2525903003/diff/1/headless/app/headless_shell...
headless/app/headless_shell.cc:176: base::Bind(&HeadlessShell::FetchTimeout,
base::Unretained(this)),
On 2016/11/23 18:11:32, Sami wrote:
> Should we use a weak pointer here? Nothing seems to prevent this from firing
> after the shell is gone.

Done.

https://codereview.chromium.org/2525903003/diff/1/headless/lib/browser/headle...
File headless/lib/browser/headless_web_contents_impl.h (right):

https://codereview.chromium.org/2525903003/diff/1/headless/lib/browser/headle...
headless/lib/browser/headless_web_contents_impl.h:77: void Stop() override;
On 2016/11/23 18:11:32, Sami wrote:
> We'll need this in devtools too, right? Should we just add it there from the
> beginning? (Sorry but this is the question I ask myself for every API we add
> here :)

Yes we should add this to devtools.

[alex clarke (OOO till 29th), 2016-11-24 12:05:10]

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

[commit-bot: I haz the power, 2016-11-24 12:05:45]

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

[alex clarke (OOO till 29th), 2016-11-24 12:06:13]

Description was changed from

==========
Add a --timeout switch to headless_shell

Some pages don't load in a reasonable time frame so allow users to
cause the load to stop after a given timeout.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
==========

to

==========
Add a --timeout switch to headless_shell

Some pages don't load in a reasonable time frame so allow users to
cause the load to stop after a given timeout.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
==========

[alex clarke (OOO till 29th), 2016-11-24 12:06:48]

Description was changed from

==========
Add a --timeout switch to headless_shell

Some pages don't load in a reasonable time frame so allow users to
cause the load to stop after a given timeout.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
==========

to

==========
Add Page.Stop to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable time frame so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.Stop devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
==========

[alex clarke (OOO till 29th), 2016-11-24 12:07:22]

alexclarke@chromium.org changed reviewers:
+ dgozman@chromium.org, pfeldman@chromium.org

[alex clarke (OOO till 29th), 2016-11-24 12:07:22]

+dgozman@ & pfeldman@ for OWNERS review.

[Sami, 2016-11-24 12:28:01]

headless/ lgtm. Thanks for fixing the other unretaineds too!

[commit-bot: I haz the power, 2016-11-24 14:07:43]

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

[commit-bot: I haz the power, 2016-11-24 14:07:44]

Dry run: Try jobs failed on following builders:
  blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_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)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng 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)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[dgozman, 2016-11-24 17:18:24]

lgtm

https://codereview.chromium.org/2525903003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/browser_protocol.json (right):

https://codereview.chromium.org/2525903003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/browser_protocol.json:262: "name":
"stop",
Let's call this stopLoading to emphasize that we don't really stop the world
(e.g. JS is still executed).

[alex clarke (OOO till 29th), 2016-11-24 17:26:41]

Description was changed from

==========
Add Page.Stop to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable time frame so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.Stop devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
==========

to

==========
Add Page.stopLoading to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable time frame so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.Stop devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
==========

[alex clarke (OOO till 29th), 2016-11-24 17:29:01]

Description was changed from

==========
Add Page.stopLoading to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable time frame so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.Stop devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
==========

to

==========
Add Page.stopLoading to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable timeframe so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.stopLoading devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
==========

[alex clarke (OOO till 29th), 2016-11-24 17:29:36]

The CQ bit was checked by alexclarke@chromium.org

[alex clarke (OOO till 29th), 2016-11-24 17:29:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/2525903003/#ps80001
(title: "FixPage.stopLoading command description.")

[commit-bot: I haz the power, 2016-11-24 17:30:12]

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-24 17:37:31]

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

[commit-bot: I haz the power, 2016-11-24 17:37:32]

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

[alex clarke (OOO till 29th), 2016-11-25 09:43:49]

The CQ bit was checked by alexclarke@chromium.org

[commit-bot: I haz the power, 2016-11-25 09:44:06]

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-25 09:49:49]

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

[commit-bot: I haz the power, 2016-11-25 09:49:50]

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

[alex clarke (OOO till 29th), 2016-11-25 11:54:46]

I did git cl land --bypass-hooks on this because the presubmit was complaining
about the LOG(INFO) in headless/app/headless_shell.cc

In this case that's fine so I bypassed it.

https://codereview.chromium.org/2525903003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/browser_protocol.json (right):

https://codereview.chromium.org/2525903003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/browser_protocol.json:262: "name":
"stop",
On 2016/11/24 17:18:24, dgozman wrote:
> Let's call this stopLoading to emphasize that we don't really stop the world
> (e.g. JS is still executed).

Done.

[commit-bot: I haz the power, 2016-11-25 11:55:04]

Description was changed from

==========
Add Page.stopLoading to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable timeframe so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.stopLoading devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
==========

to

==========
Add Page.stopLoading to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable timeframe so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.stopLoading devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
R=dgozman@chromium.org, skyostil@chromium.org

Committed: https://crrev.com/6f5c58a6767e018775b02060009a432ea81a74db
Cr-Commit-Position: refs/heads/master@{#434480}
==========

[commit-bot: I haz the power, 2016-11-25 11:55:05]

Patchset 5 (id:??) landed as
https://crrev.com/6f5c58a6767e018775b02060009a432ea81a74db
Cr-Commit-Position: refs/heads/master@{#434480}

[alex clarke (OOO till 29th), 2016-11-25 11:56:25]

Description was changed from

==========
Add Page.stopLoading to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable timeframe so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.stopLoading devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
R=dgozman@chromium.org, skyostil@chromium.org

Committed: https://crrev.com/6f5c58a6767e018775b02060009a432ea81a74db
Cr-Commit-Position: refs/heads/master@{#434480}
==========

to

==========
Add Page.stopLoading to devtools and --timeout switch to headless_shell

Some pages don't load in a reasonable timeframe so allow users to
cause the load to stop after a given timeout.  We implement this via a
new Page.stopLoading devtools command.

The new switch is --timeout=milliseconds (e.g. --timeout=5000)

BUG=546953
R=dgozman@chromium.org, skyostil@chromium.org

Committed:
https://chromium.googlesource.com/chromium/src/+/6f5c58a6767e018775b02060009a...
==========