[PresentationAPI] Ensures only one pending message callback at a time if a frame created multiple s…

[PresentationAPI] Ensures only one pending message callback at a time if a frame created multiple sessions.

Starts listening message on default session started event.

BUG=506338, 507467
Committed: https://crrev.com/76a6990391bdf0c006f47597e41cde344d376d83
Cr-Commit-Position: refs/heads/master@{#337967}

[haibinlu, 2015-07-07 17:58:24]

haibinlu@chromium.org changed reviewers:
+ avayvod@chromium.org, imcheng@chromium.org

[imcheng (use chromium acct), 2015-07-07 22:24:38]

imcheng@google.com changed reviewers:
+ imcheng@google.com

[imcheng (use chromium acct), 2015-07-07 22:24:39]

https://codereview.chromium.org/1224953002/diff/1/content/renderer/presentati...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/1224953002/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.cc:420:
DCHECK(listening_for_messages_);
Is it possible for DidCommitProvisionalLoad() to be called right before this,
which will fail the DCHECK?

https://codereview.chromium.org/1224953002/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.cc:423: if (!controller_
|| messages.is_null()) {
So, we don't use an empty list for |messages| to denote error?

[haibinlu, 2015-07-07 22:37:24]

PTAL

https://codereview.chromium.org/1224953002/diff/1/content/renderer/presentati...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/1224953002/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.cc:420:
DCHECK(listening_for_messages_);
On 2015/07/07 22:24:38, imcheng wrote:
> Is it possible for DidCommitProvisionalLoad() to be called right before this,
> which will fail the DCHECK?

good point. updated.

https://codereview.chromium.org/1224953002/diff/1/content/renderer/presentati...
content/renderer/presentation/presentation_dispatcher.cc:423: if (!controller_
|| messages.is_null()) {
On 2015/07/07 22:24:39, imcheng wrote:
> So, we don't use an empty list for |messages| to denote error?

right. no empty list. presentation_service_impl converts it to an null list.

[imcheng (use chromium acct), 2015-07-07 22:51:06]

lgtm

[haibinlu, 2015-07-08 18:05:10]

haibinlu@chromium.org changed reviewers:
+ kmarshall@chromium.org

[imcheng, 2015-07-08 20:07:48]

lgtm

[Kevin M, 2015-07-08 20:16:08]

lgtm

https://codereview.chromium.org/1224953002/diff/70001/content/renderer/presen...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/1224953002/diff/70001/content/renderer/presen...
content/renderer/presentation/presentation_dispatcher.cc:426: if
(!listening_for_messages_)
Add a LOG/DLOG here to call our attention to strange behavior.

https://codereview.chromium.org/1224953002/diff/70001/content/renderer/presen...
content/renderer/presentation/presentation_dispatcher.cc:431:
listening_for_messages_ = false;
LOG?

[haibinlu, 2015-07-08 20:47:42]

https://codereview.chromium.org/1224953002/diff/70001/content/renderer/presen...
File content/renderer/presentation/presentation_dispatcher.cc (right):

https://codereview.chromium.org/1224953002/diff/70001/content/renderer/presen...
content/renderer/presentation/presentation_dispatcher.cc:426: if
(!listening_for_messages_)
On 2015/07/08 20:16:08, Kevin M wrote:
> Add a LOG/DLOG here to call our attention to strange behavior.

added comments.

this may happen. frame navigated, and MR sent last batches of messages; where
there may be race condition.

https://codereview.chromium.org/1224953002/diff/70001/content/renderer/presen...
content/renderer/presentation/presentation_dispatcher.cc:431:
listening_for_messages_ = false;
On 2015/07/08 20:16:07, Kevin M wrote:
> LOG?

null message is expected to indicate messaging error or the end of messaging
channel due to route removal. Log is not really necessary.

[haibinlu, 2015-07-08 20:49:16]

The CQ bit was checked by haibinlu@chromium.org

[haibinlu, 2015-07-08 20:49:17]

The patchset sent to the CQ was uploaded after l-g-t-m from imcheng@google.com,
imcheng@chromium.org, kmarshall@chromium.org
Link to the patchset: https://codereview.chromium.org/1224953002/#ps90001
(title: " ")

[commit-bot: I haz the power, 2015-07-08 20:51:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1224953002/90001

[commit-bot: I haz the power, 2015-07-08 22:12:16]

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

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

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

[haibinlu, 2015-07-09 00:59:27]

The CQ bit was checked by haibinlu@chromium.org

[haibinlu, 2015-07-09 00:59:27]

The patchset sent to the CQ was uploaded after l-g-t-m from imcheng@google.com,
imcheng@chromium.org, kmarshall@chromium.org
Link to the patchset: https://codereview.chromium.org/1224953002/#ps110001
(title: "rebase")

[commit-bot: I haz the power, 2015-07-09 01:00:53]

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

[commit-bot: I haz the power, 2015-07-09 02:12:47]

Committed patchset #7 (id:110001)

[commit-bot: I haz the power, 2015-07-09 02:13:36]

Patchset 7 (id:??) landed as
https://crrev.com/76a6990391bdf0c006f47597e41cde344d376d83
Cr-Commit-Position: refs/heads/master@{#337967}