Remove obsolete StoragePartition check from old iframe signin path.

Remove obsolete StoragePartition check from old iframe signin path.

For now, leave the UI thread hop in place for DevTools URLs, due
to issue 616641.

BUG=614808, 616282, 616641
TEST=Signin and Mac DevTools still work.

Committed: https://crrev.com/f5a831f23ed6fdfed5507e5ce9766a7df5d4203c
Cr-Commit-Position: refs/heads/master@{#397530}

[Charlie Reis, 2016-06-02 00:49:29]

Patchset #1 (id:1) has been deleted

[Charlie Reis, 2016-06-02 01:02:16]

Description was changed from

==========
Remove obsolete StoragePartition check from old iframe signin path.

For now, leave the UI thread hop in place for DevTools URLs, due
to issue NNNNNN.

BUG=614808
TEST=Signin and DevTools on Mac still work.
==========

to

==========
Remove obsolete StoragePartition check from old iframe signin path.

For now, leave the UI thread hop in place for DevTools URLs, due
to issue 616641.

BUG=614808, 616282, 616641
TEST=Signin and DevTools on Mac still work.
==========

[Charlie Reis, 2016-06-02 16:38:20]

Description was changed from

==========
Remove obsolete StoragePartition check from old iframe signin path.

For now, leave the UI thread hop in place for DevTools URLs, due
to issue 616641.

BUG=614808, 616282, 616641
TEST=Signin and DevTools on Mac still work.
==========

to

==========
Remove obsolete StoragePartition check from old iframe signin path.

For now, leave the UI thread hop in place for DevTools URLs, due
to issue 616641.

BUG=614808, 616282, 616641
TEST=Signin and Mac DevTools still work.
==========

[Charlie Reis, 2016-06-02 16:38:20]

creis@chromium.org changed reviewers:
+ caseq@chromium.org, dbeam@chromium.org

[Charlie Reis, 2016-06-02 16:42:11]

dbeam@ and caseq@, can you review?  This is the same as
https://codereview.chromium.org/2007223003 except that it leaves the UI thread
hop in place for DevTools URLs.  (I also remembered to remove the method from
ContentBrowserClient in content/public this time.)

There's a recently added linux_android_rel_ng test that was failing in a try
job, but I'm running again to see if it's a flake or a real issue.  I'd be
surprised if it's real, given how little time has passed since the revert.

CC'ing anthonyvd@ for FYI.

https://codereview.chromium.org/2033563002/diff/40001/content/browser/webui/u...
File content/browser/webui/url_data_manager_backend.cc (right):

https://codereview.chromium.org/2033563002/diff/40001/content/browser/webui/u...
content/browser/webui/url_data_manager_backend.cc:187: // TODO(caseq): Fix the
race condition and remove this thread hop in
caseq@: Note that the TODO(username) convention doesn't mean you need to fix it,
but just that you have some knowledge about it in case others have questions.

[Charlie Reis, 2016-06-02 17:30:24]

On 2016/06/02 16:42:11, Charlie Reis wrote:
> There's a recently added linux_android_rel_ng test that was failing in a try
> job, but I'm running again to see if it's a flake or a real issue.  I'd be
> surprised if it's real, given how little time has passed since the revert.

Ah good, it went green.

[Dan Beam, 2016-06-02 18:58:07]

https://codereview.chromium.org/2033563002/diff/40001/content/browser/webui/u...
File content/browser/webui/url_data_manager_backend.cc (right):

https://codereview.chromium.org/2033563002/diff/40001/content/browser/webui/u...
content/browser/webui/url_data_manager_backend.cc:278:
weak_factory_.GetWeakPtr()));
did you meant to put a "return;" here?

[Charlie Reis, 2016-06-02 19:01:42]

Thanks.  Sadly, I couldn't find a Mac try bot that runs the DevToolsSanityTests
(which would have caught that), but I'll watch those bots as soon as it lands
and revert if it doesn't work.  Fixed.

https://codereview.chromium.org/2033563002/diff/40001/content/browser/webui/u...
File content/browser/webui/url_data_manager_backend.cc (right):

https://codereview.chromium.org/2033563002/diff/40001/content/browser/webui/u...
content/browser/webui/url_data_manager_backend.cc:278:
weak_factory_.GetWeakPtr()));
On 2016/06/02 18:58:07, Dan Beam wrote:
> did you meant to put a "return;" here?

Yes!  Good catch.

[Charlie Reis, 2016-06-02 19:01:48]

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

[commit-bot: I haz the power, 2016-06-02 19:03:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2033563002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2033563002/60001

[Dan Beam, 2016-06-02 19:08:21]

if you realllly care, you could probably write a test that checks the current
behavior posts task to the expected threads and such, but up to you (I've never
written a test like that and wouldn't really know where to start all too much)

either way, lgtm

[Charlie Reis, 2016-06-02 19:34:57]

On 2016/06/02 19:08:21, Dan Beam wrote:
> if you realllly care, you could probably write a test that checks the current
> behavior posts task to the expected threads and such, but up to you (I've
never
> written a test like that and wouldn't really know where to start all too much)
> 
> either way, lgtm

Yeah, that seems more like testing the workaround than the feature it's for. 
The good thing about the DevToolsSanityTests is that they'll continue to work
when the race condition is fixed and thread hop is removed.

caseq@: I'll proceed with this and keep an eye on
https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests.

[caseq, 2016-06-02 20:39:48]

lgtm

[Charlie Reis, 2016-06-02 20:42:22]

The CQ bit was unchecked by creis@chromium.org

[Charlie Reis, 2016-06-02 20:42:27]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2016-06-02 20:42:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2033563002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2033563002/60001

[commit-bot: I haz the power, 2016-06-02 22:24:32]

Description was changed from

==========
Remove obsolete StoragePartition check from old iframe signin path.

For now, leave the UI thread hop in place for DevTools URLs, due
to issue 616641.

BUG=614808, 616282, 616641
TEST=Signin and Mac DevTools still work.
==========

to

==========
Remove obsolete StoragePartition check from old iframe signin path.

For now, leave the UI thread hop in place for DevTools URLs, due
to issue 616641.

BUG=614808, 616282, 616641
TEST=Signin and Mac DevTools still work.
==========

[commit-bot: I haz the power, 2016-06-02 22:24:33]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-06-02 22:26:21]

Description was changed from

==========
Remove obsolete StoragePartition check from old iframe signin path.

For now, leave the UI thread hop in place for DevTools URLs, due
to issue 616641.

BUG=614808, 616282, 616641
TEST=Signin and Mac DevTools still work.
==========

to

==========
Remove obsolete StoragePartition check from old iframe signin path.

For now, leave the UI thread hop in place for DevTools URLs, due
to issue 616641.

BUG=614808, 616282, 616641
TEST=Signin and Mac DevTools still work.

Committed: https://crrev.com/f5a831f23ed6fdfed5507e5ce9766a7df5d4203c
Cr-Commit-Position: refs/heads/master@{#397530}
==========

[commit-bot: I haz the power, 2016-06-02 22:26:22]

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