Restrict number of BlimpContents to [0, 1]

Restrict number of BlimpContents to [0, 1]

Currently, if CreateBlimpContents() is called more than once, multiple
BlimpContents are created with the same kDummyTabId. In this CL,
1. BlimpContentsManager will return nullptr if CreateBlimpContents() is
called when there's an existing BlimpContents.
2. If BlimpContentsManager returns nullptr, BlimpClientContextImpl won't
set the delegate.
2. TabBlimpContentsObserver won't be added in Tab.java

BUG=642216
Committed: https://crrev.com/10044bb32baafa8bb90026bb30e66387822d7102
Cr-Commit-Position: refs/heads/master@{#415532}

[Menglin, 2016-08-30 23:03:06]

mlliu@chromium.org changed reviewers:
+ dtrainor@chromium.org

[Menglin, 2016-08-30 23:03:08]

HI David, please review this change. Thanks!

Menglin

[David Trainor- moved to gerrit, 2016-08-30 23:31:12]

lgtm with suggestion/nits

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
File blimp/client/core/contents/blimp_contents_manager.h (right):

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
blimp/client/core/contents/blimp_contents_manager.h:62: bool tab_exists_ =
false;
Can we just check the map to see if any entries exist?

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
File blimp/client/core/contents/blimp_contents_manager_unittest.cc (right):

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
blimp/client/core/contents/blimp_contents_manager_unittest.cc:85:
TEST(BlimpContentsManagerUnittest, CreateTwoBlimpContentsDestroyAndCreate) {
Add a TODO and a bug to remove this test and the related code once we support
more than one BlimpContents.

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
blimp/client/core/contents/blimp_contents_manager_unittest.cc:94:
DCHECK(blimp_contents);
EXPECT_NE()?  Same below.

https://codereview.chromium.org/2299453002/diff/1/blimp/client/public/blimp_c...
File blimp/client/public/blimp_client_context.h (right):

https://codereview.chromium.org/2299453002/diff/1/blimp/client/public/blimp_c...
blimp/client/public/blimp_client_context.h:55: // Currently we want to have a
single BlimpContents. If there is an existing
Add the same TODO.

[Menglin, 2016-08-31 00:13:02]

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
File blimp/client/core/contents/blimp_contents_manager.h (right):

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
blimp/client/core/contents/blimp_contents_manager.h:62: bool tab_exists_ =
false;
On 2016/08/30 23:31:12, David Trainor wrote:
> Can we just check the map to see if any entries exist?

There can be such case:
https://codesearch.chromium.org/chromium/src/blimp/client/core/contents/blimp...
, i.e., the contents is destroyed but the observer map is not empty yet. So I
figured it's more straightforward to just use a bool.

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
File blimp/client/core/contents/blimp_contents_manager_unittest.cc (right):

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
blimp/client/core/contents/blimp_contents_manager_unittest.cc:85:
TEST(BlimpContentsManagerUnittest, CreateTwoBlimpContentsDestroyAndCreate) {
On 2016/08/30 23:31:12, David Trainor wrote:
> Add a TODO and a bug to remove this test and the related code once we support
> more than one BlimpContents.

Done.

https://codereview.chromium.org/2299453002/diff/1/blimp/client/core/contents/...
blimp/client/core/contents/blimp_contents_manager_unittest.cc:94:
DCHECK(blimp_contents);
On 2016/08/30 23:31:12, David Trainor wrote:
> EXPECT_NE()?  Same below.

Done.

https://codereview.chromium.org/2299453002/diff/1/blimp/client/public/blimp_c...
File blimp/client/public/blimp_client_context.h (right):

https://codereview.chromium.org/2299453002/diff/1/blimp/client/public/blimp_c...
blimp/client/public/blimp_client_context.h:55: // Currently we want to have a
single BlimpContents. If there is an existing
On 2016/08/30 23:31:12, David Trainor wrote:
> Add the same TODO.

Done.

[Menglin, 2016-08-31 00:13:07]

The CQ bit was checked by mlliu@chromium.org

[Menglin, 2016-08-31 00:13:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2299453002/#ps20001
(title: "nits")

[commit-bot: I haz the power, 2016-08-31 00:13:56]

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-31 00:45:35]

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

[commit-bot: I haz the power, 2016-08-31 00:45:36]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[Menglin, 2016-08-31 00:56:09]

The CQ bit was checked by mlliu@chromium.org

[Menglin, 2016-08-31 00:56:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2299453002/#ps40001
(title: "patch failure fix")

[commit-bot: I haz the power, 2016-08-31 00:56:30]

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-31 01:38:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-31 01:43:13]

Description was changed from

==========
Restrict number of BlimpContents to [0, 1]

Currently, if CreateBlimpContents() is called more than once, multiple
BlimpContents are created with the same kDummyTabId. In this CL,
1. BlimpContentsManager will return nullptr if CreateBlimpContents() is
called when there's an existing BlimpContents.
2. If BlimpContentsManager returns nullptr, BlimpClientContextImpl won't
set the delegate.
2. TabBlimpContentsObserver won't be added in Tab.java

BUG=642216
==========

to

==========
Restrict number of BlimpContents to [0, 1]

Currently, if CreateBlimpContents() is called more than once, multiple
BlimpContents are created with the same kDummyTabId. In this CL,
1. BlimpContentsManager will return nullptr if CreateBlimpContents() is
called when there's an existing BlimpContents.
2. If BlimpContentsManager returns nullptr, BlimpClientContextImpl won't
set the delegate.
2. TabBlimpContentsObserver won't be added in Tab.java

BUG=642216

Committed: https://crrev.com/10044bb32baafa8bb90026bb30e66387822d7102
Cr-Commit-Position: refs/heads/master@{#415532}
==========

[commit-bot: I haz the power, 2016-08-31 01:43:14]

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