Add mechanism to get the Tab from the associated WebState.

Add mechanism to get the Tab from the associated WebState.

BUG=687207
Review-Url: https://codereview.chromium.org/2681843002
Cr-Commit-Position: refs/heads/master@{#449274}
Committed: https://chromium.googlesource.com/chromium/src/+/75ec07fd18d0f575744802610ffba4d021748022

[sdefresne, 2017-02-07 18:09:43]

sdefresne@chromium.org changed reviewers:
+ rohitrao@chromium.org

[sdefresne, 2017-02-07 18:09:43]

Please take a look.

[rohitrao (ping after 24h), 2017-02-08 14:07:17]

lgtm

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/leg...
File ios/chrome/browser/tabs/legacy_tab_helper.h (right):

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/leg...
ios/chrome/browser/tabs/legacy_tab_helper.h:14: class LegacyTabHelper : public
web::WebStateUserData<LegacyTabHelper> {
Class could use a comment explaining what it does and that it's only needed for
migration.

Please add comments to the getter and ivars too.

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/tab.mm
File ios/chrome/browser/tabs/tab.mm (right):

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/tab...
ios/chrome/browser/tabs/tab.mm:1257:
LegacyTabHelper::RemoveFromWebState(webStateImpl_.get());
Is there any way for code to call LegacyTabHelper::FromWebState() after this
line is run?  I don't want to require null-checking.  If it's basically
impossible for code to run after this call, then that's ok.  (Otherwise we
should consider calling SetTab(nil) instead of removing the helper completely.)

[rohitrao (ping after 24h), 2017-02-08 14:07:48]

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/BUI...
File ios/chrome/browser/tabs/BUILD.gn (right):

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/BUI...
ios/chrome/browser/tabs/BUILD.gn:33: "legacy_tab_helper.h",
Should we make this an ARC file?

[sdefresne, 2017-02-08 14:44:42]

Patchset #2 (id:20001) has been deleted

[sdefresne, 2017-02-08 14:45:34]

PTAL

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/BUI...
File ios/chrome/browser/tabs/BUILD.gn (right):

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/BUI...
ios/chrome/browser/tabs/BUILD.gn:33: "legacy_tab_helper.h",
On 2017/02/08 14:07:48, rohitrao wrote:
> Should we make this an ARC file?

Done.

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/leg...
File ios/chrome/browser/tabs/legacy_tab_helper.h (right):

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/leg...
ios/chrome/browser/tabs/legacy_tab_helper.h:14: class LegacyTabHelper : public
web::WebStateUserData<LegacyTabHelper> {
On 2017/02/08 14:07:17, rohitrao wrote:
> Class could use a comment explaining what it does and that it's only needed
for
> migration.
> 
> Please add comments to the getter and ivars too.

Done.

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/tab.mm
File ios/chrome/browser/tabs/tab.mm (right):

https://codereview.chromium.org/2681843002/diff/1/ios/chrome/browser/tabs/tab...
ios/chrome/browser/tabs/tab.mm:1257:
LegacyTabHelper::RemoveFromWebState(webStateImpl_.get());
On 2017/02/08 14:07:17, rohitrao wrote:
> Is there any way for code to call LegacyTabHelper::FromWebState() after this
> line is run?  I don't want to require null-checking.  If it's basically
> impossible for code to run after this call, then that's ok.  (Otherwise we
> should consider calling SetTab(nil) instead of removing the helper
completely.)

I've addressed this by introducing a static method
LegacyTabHelper::GetTabForWebState() that does the null check.

[sdefresne, 2017-02-08 14:45:43]

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

[commit-bot: I haz the power, 2017-02-08 14:45:59]

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

[rohitrao (ping after 24h), 2017-02-08 14:54:23]

lgtm

[commit-bot: I haz the power, 2017-02-08 16:03:03]

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

[commit-bot: I haz the power, 2017-02-08 16:03:04]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2017-02-09 13:02:58]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2017-02-09 13:03:17]

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-09 13:08:18]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486645378556530,
"parent_rev": "57720137ab43483ab50510fb7f4db7cc0cad412f", "commit_rev":
"75ec07fd18d0f575744802610ffba4d021748022"}

[commit-bot: I haz the power, 2017-02-09 13:08:58]

Description was changed from

==========
Add mechanism to get the Tab from the associated WebState.

BUG=687207
==========

to

==========
Add mechanism to get the Tab from the associated WebState.

BUG=687207

Review-Url: https://codereview.chromium.org/2681843002
Cr-Commit-Position: refs/heads/master@{#449274}
Committed:
https://chromium.googlesource.com/chromium/src/+/75ec07fd18d0f575744802610ffb...
==========

[commit-bot: I haz the power, 2017-02-09 13:09:00]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/75ec07fd18d0f575744802610ffb...