content: Don't keep requesting the GpuChannelHost during shutdown.

content: Don't keep requesting the GpuChannelHost during shutdown.

During shutdown, the Gpu Channel initialization returns a nullptr.
Early out in that case rather than keep queueing more requests.

BUG=636294
Committed: https://crrev.com/3e1eed893f3fe23256d5c418eecfc72ddd95fb8b
Cr-Commit-Position: refs/heads/master@{#411254}

[Khushal, 2016-08-11 01:26:34]

khushalsagar@chromium.org changed reviewers:
+ piman@chromium.org

[Khushal, 2016-08-11 01:31:05]

On 2016/08/11 01:26:34, Khushal wrote:

Seems to me that the better thing would be to move the timeout logic to the
BrowserGpuChannelHostFactory instead and drop the callbacks during shutdown. I
can do that in a follow up patch, could you take a quick look at this since its
breaking a lot of bots.

[piman, 2016-08-11 01:37:56]

lgtm

[Khushal, 2016-08-11 01:43:41]

The CQ bit was checked by khushalsagar@chromium.org

[commit-bot: I haz the power, 2016-08-11 01:44:40]

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-08-11 03:38:47]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-11 03:40:43]

Description was changed from

==========
content: Don't keep requesting the GpuChannelHost during shutdown.

During shutdown, the Gpu Channel initialization returns a nullptr.
Early out in that case rather than keep queueing more requests.

BUG=636294
==========

to

==========
content: Don't keep requesting the GpuChannelHost during shutdown.

During shutdown, the Gpu Channel initialization returns a nullptr.
Early out in that case rather than keep queueing more requests.

BUG=636294

Committed: https://crrev.com/3e1eed893f3fe23256d5c418eecfc72ddd95fb8b
Cr-Commit-Position: refs/heads/master@{#411254}
==========

[commit-bot: I haz the power, 2016-08-11 03:40:44]

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

[no sievers, 2016-08-11 18:15:22]

Summarizing a few thoughts from offline discussion:

1) It would be nicer to change this:

void BrowserGpuChannelHostFactory::Terminate() {
  DCHECK(instance_);
+  instance_ = NULL;
  delete instance_;
-  instance_ = NULL;
}

So that we can avoid reentrancy from the callback during factory destruction and
queuing more requests (and infinite recursion), while also making sure the
caller handles BGCHF::instance() == null everywhere.

2) Somewhere we have to trigger the retry. But that makes me wonder if we had
been handling this poorly.
In particular if we have become invisible and don't really need to keep trying
until we become visible again.
And it could explain some weird gpu channel establishment timeout scenarios (for
which we have crash reports).

Maybe the easiest thing is to deliver the failure (null host in general - which
could be because of factory teardown but more importantly because of some real
GPU process (re)launch failure) to the caller (CompositorImpl) always and then
let it decide there if it's a good time to retry (based on visibility and
surface availability).

[no sievers, 2016-08-11 18:19:26]

Description was changed from

==========
content: Don't keep requesting the GpuChannelHost during shutdown.

During shutdown, the Gpu Channel initialization returns a nullptr.
Early out in that case rather than keep queueing more requests.

BUG=636294

Committed: https://crrev.com/3e1eed893f3fe23256d5c418eecfc72ddd95fb8b
Cr-Commit-Position: refs/heads/master@{#411254}
==========

to

==========
content: Don't keep requesting the GpuChannelHost during shutdown.

During shutdown, the Gpu Channel initialization returns a nullptr.
Early out in that case rather than keep queueing more requests.

BUG=636294

Committed: https://crrev.com/3e1eed893f3fe23256d5c418eecfc72ddd95fb8b
Cr-Commit-Position: refs/heads/master@{#411254}
==========