Removing references to Off the Record from MediaRouter

Removing references to Off the Record from comments and source code related to Media Router

BUG=3333
R=tommycli@chromium.org
TBR=

Committed: https://crrev.com/9f8e900ba18c0e8a76b9b9e07dc51f0e9e4dcb72
Cr-Commit-Position: refs/heads/master@{#406968}

[trizzofo, 2016-06-29 20:53:56]

tommycli, ptal

[tommycli, 2016-06-29 23:09:33]

lgtm except one nitpick:

https://codereview.chromium.org/2112673002/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/media_route.h (right):

https://codereview.chromium.org/2112673002/diff/1/chrome/browser/media/router...
chrome/browser/media/router/media_route.h:77: // Sets incognito.  Set this to
true when the route was created by an
I recommend refactoring this comment to simply say 
"Set this to true when the route was created by an incognito profile.", since
the first sentence is obvious.

[trizzofo, 2016-06-29 23:32:01]

Description was changed from

==========
Removing references to Off the Record from comments and source code
related to Media Router

BUG=3333

R=tommycli@chromium.org
==========

to

==========
Removing references to Off the Record from comments and source code related to
Media Router

BUG=3333

R=tommycli@chromium.org
==========

[trizzofo, 2016-06-29 23:32:22]

https://codereview.chromium.org/2112673002/diff/1/chrome/browser/media/router...
File chrome/browser/media/router/media_route.h (right):

https://codereview.chromium.org/2112673002/diff/1/chrome/browser/media/router...
chrome/browser/media/router/media_route.h:77: // Sets incognito.  Set this to
true when the route was created by an
On 2016/06/29 23:09:33, tommycli wrote:
> I recommend refactoring this comment to simply say 
> "Set this to true when the route was created by an incognito profile.", since
> the first sentence is obvious.

Done.

[trizzofo, 2016-06-29 23:35:38]

trizzofo@google.com changed reviewers:
+ apacible@chromium.org

[trizzofo, 2016-06-29 23:39:09]

The CQ bit was checked by trizzofo@google.com to run a CQ dry run

[trizzofo, 2016-06-29 23:39:41]

The CQ bit was unchecked by trizzofo@google.com

[trizzofo, 2016-06-29 23:40:47]

The CQ bit was checked by trizzofo@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-29 23:41:39]

Dry run: 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-06-29 23:53:52]

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

[commit-bot: I haz the power, 2016-06-29 23:53:53]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[trizzofo, 2016-06-30 00:55:03]

The CQ bit was checked by trizzofo@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 00:55:49]

Dry run: 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-06-30 01:04:33]

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

[commit-bot: I haz the power, 2016-06-30 01:04:34]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[trizzofo, 2016-06-30 17:51:08]

The CQ bit was checked by trizzofo@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-30 17:51:43]

Dry run: 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-06-30 17:56:20]

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

[commit-bot: I haz the power, 2016-06-30 17:56:21]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[trizzofo, 2016-06-30 22:57:59]

apacible, PTAL

[apacible, 2016-07-06 17:32:59]

Do you plan on renaming the rest of the Media Router files as well? It would be
great if everything was in a single CL to keep it uniform.

[trizzofo, 2016-07-06 21:37:12]

The CQ bit was checked by trizzofo@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-06 21:38:35]

Dry run: 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-07-06 21:48:05]

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

[commit-bot: I haz the power, 2016-07-06 21:48:06]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[trizzofo, 2016-07-07 18:39:39]

I renamed occurrences found in some other files.
Let me know if I'm missing something.

[apacible, 2016-07-09 06:07:28]

We're missing extensions/renderer/resources/media_router_bindings.js.

There's an extension side change (not in chromium) we'll have to make since
there will be some naming mismatches. Please leave the offTheRecord as is on the
RHS of line 81 of the bindings file; I'll update that separately to keep things
in sync. Everything else can be updated to incognito.

You'll also want someone on the media/android/router OWNERS to review.

https://codereview.chromium.org/2112673002/diff/80001/chrome/browser/media/ro...
File chrome/browser/media/router/media_router_factory_unittest.cc (right):

https://codereview.chromium.org/2112673002/diff/80001/chrome/browser/media/ro...
chrome/browser/media/router/media_router_factory_unittest.cc:59: // Creates an
incognito (off the record) profile.
nit: "(off the record)" can be removed.

[trizzofo, 2016-07-11 18:05:12]

The CQ bit was checked by trizzofo@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-11 18:05:32]

Dry run: 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-07-11 18:16:03]

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

[commit-bot: I haz the power, 2016-07-11 18:16:04]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[trizzofo, 2016-07-11 19:20:30]

trizzofo@google.com changed reviewers:
+ avayvod@chromium.org

[trizzofo, 2016-07-11 19:20:31]

Thanks apacible

avayvod PTAL

https://codereview.chromium.org/2112673002/diff/80001/chrome/browser/media/ro...
File chrome/browser/media/router/media_router_factory_unittest.cc (right):

https://codereview.chromium.org/2112673002/diff/80001/chrome/browser/media/ro...
chrome/browser/media/router/media_router_factory_unittest.cc:59: // Creates an
incognito (off the record) profile.
On 2016/07/09 06:07:28, apacible wrote:
> nit: "(off the record)" can be removed.

Done.

[apacible, 2016-07-11 20:08:10]

apacible@chromium.org changed reviewers:
+ imcheng@chromium.org

[apacible, 2016-07-11 20:08:11]

+imcheng for bindings OWNERS

[apacible, 2016-07-11 20:10:17]

lgtm

A quick tip for future CLs -- When there are other code changes, e.g. someone
else's CL lands before your CL, upload their changes in a separate patch and
label it something like "Rebase". It'll make it easier to figure out what the
changes are between patches that need review. :)

Here's an example where I upload "Rebase" patches separately from my own
changes: https://codereview.chromium.org/1868983003/

[imcheng, 2016-07-11 20:10:24]

media_router_bindings.js lgtm

[apacible, 2016-07-11 20:14:42]

FYI filed crbug/627213 for that last renaming.

[trizzofo, 2016-07-11 20:35:07]

On 2016/07/11 20:10:17, apacible wrote:
> lgtm
> 
> A quick tip for future CLs -- When there are other code changes, e.g. someone
> else's CL lands before your CL, upload their changes in a separate patch and
> label it something like "Rebase". It'll make it easier to figure out what the
> changes are between patches that need review. :)
> 
> Here's an example where I upload "Rebase" patches separately from my own
> changes: https://codereview.chromium.org/1868983003/

I'll start doing that. Thanks for the tip!

[whywhat, 2016-07-12 10:54:06]

avayvod@chromium.org changed reviewers:
+ tommycli@chromium.or
- tommycli@chromium.org

[whywhat, 2016-07-12 10:54:07]

*android* lgtm

[trizzofo, 2016-07-12 17:14:45]

The CQ bit was checked by trizzofo@google.com

[trizzofo, 2016-07-12 17:14:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org
Link to the patchset: https://codereview.chromium.org/2112673002/#ps100001
(title: "Nit fixed and more refactoring")

[commit-bot: I haz the power, 2016-07-12 17:16:43]

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-07-12 17:29:16]

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

[commit-bot: I haz the power, 2016-07-12 17:29:18]

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

[trizzofo, 2016-07-13 19:05:25]

trizzofo@google.com changed reviewers:
+ tommycli@chromium.org, tsepez@chromium.org
- tommycli@chromium.or

[trizzofo, 2016-07-13 19:05:26]

tsepez PTAL at
chrome/browser/media/router/mojo/media_router.mojom
chrome/browser/media/router/mojo/media_router_type_converters.cc
chrome/browser/media/router/mojo/media_router_type_converters_unittest.cc

[Tom Sepez, 2016-07-13 19:07:48]

LGTM.  Thanks for the cleanup.

[trizzofo, 2016-07-13 19:08:54]

The CQ bit was checked by trizzofo@google.com

[commit-bot: I haz the power, 2016-07-13 19:10:32]

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-07-13 19:26:20]

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

[commit-bot: I haz the power, 2016-07-13 19:26:23]

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

[tommycli, 2016-07-13 20:25:25]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2016-07-13 20:26:41]

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-07-13 20:40:32]

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

[commit-bot: I haz the power, 2016-07-13 20:40:34]

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

[tommycli, 2016-07-15 23:17:52]

Description was changed from

==========
Removing references to Off the Record from comments and source code related to
Media Router

BUG=3333

R=tommycli@chromium.org
==========

to

==========
Removing references to Off the Record from comments and source code related to
Media Router

BUG=3333

R=tommycli@chromium.org
TBR=
==========

[tommycli, 2016-07-15 23:18:17]

Presubmit error odd since tsepez has already LGed this patch. I'm sending it in
with a TBR=.

[tommycli, 2016-07-15 23:18:26]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2016-07-15 23:19:06]

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-07-15 23:27:59]

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

[commit-bot: I haz the power, 2016-07-15 23:28:01]

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

[tommycli, 2016-07-15 23:33:27]

tommycli@chromium.org changed reviewers:
+ dcheng@chromium.org

[tommycli, 2016-07-15 23:33:28]

dcheng@ (or tsepez) - Do you guys know why this is failing security presubmit
even though tsepez has stamped it?

Thanks,
Tommy

[dcheng, 2016-07-18 05:01:00]

On 2016/07/15 23:33:28, tommycli wrote:
> dcheng@ (or tsepez) - Do you guys know why this is failing security presubmit
> even though tsepez has stamped it?
> 
> Thanks,
> Tommy

From the PRESUBMIT.py output:

chrome/browser/media/router/mojo/OWNERS is missing the following lines:

per-file *.mojom=set noparent
per-file *.mojom=file://ipc/SECURITY_OWNERS
per-file *_type_converter*.*=set noparent
per-file *_type_converter*.*=file://ipc/SECURITY_OWNERS

So the OWNERS files needs to be updated to match the patterns the check is
looking for.

Unfortunately, it looks like these lines don't exist at all atm: in fact, I'm
doing a general review of the media router mojom in another CL.

[trizzofo, 2016-07-18 20:31:19]

Patchset #7 (id:120001) has been deleted

[Wez, 2016-07-18 23:33:39]

FYI, posted https://codereview.chromium.org/2154303003 to fix OWNERS

On 17 July 2016 at 22:01, <dcheng@chromium.org> wrote:

> On 2016/07/15 23:33:28, tommycli wrote:
> > dcheng@ (or tsepez) - Do you guys know why this is failing security
> presubmit
> > even though tsepez has stamped it?
> >
> > Thanks,
> > Tommy
>
> From the PRESUBMIT.py output:
>
> chrome/browser/media/router/mojo/OWNERS is missing the following lines:
>
> per-file *.mojom=set noparent
> per-file *.mojom=file://ipc/SECURITY_OWNERS
> per-file *_type_converter*.*=set noparent
> per-file *_type_converter*.*=file://ipc/SECURITY_OWNERS
>
> So the OWNERS files needs to be updated to match the patterns the check is
> looking for.
>
> Unfortunately, it looks like these lines don't exist at all atm: in fact,
> I'm
> doing a general review of the media router mojom in another CL.
>
> https://codereview.chromium.org/2112673002/
>
> --
> You received this message because you are subscribed to the Google Groups
> "Media Router Team" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to media-router+unsubscribe@chromium.org.
> To post to this group, send email to media-router@chromium.org.
> To view this discussion on the web visit
>
https://groups.google.com/a/chromium.org/d/msgid/media-router/001a114da244c03...
>
<https://groups.google.com/a/chromium.org/d/msgid/media-router/001a114da244c03...>
> .
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[trizzofo, 2016-07-18 23:54:34]

On 2016/07/18 23:33:39, Wez wrote:
> FYI, posted https://codereview.chromium.org/2154303003 to fix OWNERS
> 
> On 17 July 2016 at 22:01, <mailto:dcheng@chromium.org> wrote:
> 
> > On 2016/07/15 23:33:28, tommycli wrote:
> > > dcheng@ (or tsepez) - Do you guys know why this is failing security
> > presubmit
> > > even though tsepez has stamped it?
> > >
> > > Thanks,
> > > Tommy
> >
> > From the PRESUBMIT.py output:
> >
> > chrome/browser/media/router/mojo/OWNERS is missing the following lines:
> >
> > per-file *.mojom=set noparent
> > per-file *.mojom=file://ipc/SECURITY_OWNERS
> > per-file *_type_converter*.*=set noparent
> > per-file *_type_converter*.*=file://ipc/SECURITY_OWNERS
> >
> > So the OWNERS files needs to be updated to match the patterns the check is
> > looking for.
> >
> > Unfortunately, it looks like these lines don't exist at all atm: in fact,
> > I'm
> > doing a general review of the media router mojom in another CL.
> >
> > https://codereview.chromium.org/2112673002/
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "Media Router Team" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to mailto:media-router+unsubscribe@chromium.org.
> > To post to this group, send email to mailto:media-router@chromium.org.
> > To view this discussion on the web visit
> >
>
https://groups.google.com/a/chromium.org/d/msgid/media-router/001a114da244c03...
> >
>
<https://groups.google.com/a/chromium.org/d/msgid/media-router/001a114da244c03...>
> > .
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Thanks!

[trizzofo, 2016-07-21 17:38:59]

The CQ bit was checked by trizzofo@google.com

[trizzofo, 2016-07-21 17:39:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org
Link to the patchset: https://codereview.chromium.org/2112673002/#ps100001
(title: "Nit fixed and more refactoring")

[commit-bot: I haz the power, 2016-07-21 17:39:32]

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-07-21 18:25:54]

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

[commit-bot: I haz the power, 2016-07-21 18:25:58]

Failed to apply patch for chrome/browser/media/router/route_request_result.h:
While running git apply --index -3 -p1;
  error: patch failed: chrome/browser/media/router/route_request_result.h:36
  Falling back to three-way merge...
  Applied patch to 'chrome/browser/media/router/route_request_result.h' with
conflicts.
  U chrome/browser/media/router/route_request_result.h

Patch:       chrome/browser/media/router/route_request_result.h
Index: chrome/browser/media/router/route_request_result.h
diff --git a/chrome/browser/media/router/route_request_result.h
b/chrome/browser/media/router/route_request_result.h
index
0a473a41e19ca79b204048a25a03b4ed387edd7e..b066dcf03fb34cc75b075620a1fb300f4f134a90
100644
--- a/chrome/browser/media/router/route_request_result.h
+++ b/chrome/browser/media/router/route_request_result.h
@@ -36,7 +36,7 @@ class RouteRequestResult {
     TIMED_OUT,
     ROUTE_NOT_FOUND,
     INVALID_ORIGIN,
-    OFF_THE_RECORD_MISMATCH
+    INCOGNITO_MISMATCH
   };
 
   static std::unique_ptr<RouteRequestResult> FromSuccess(

[trizzofo, 2016-07-21 21:23:09]

The CQ bit was checked by trizzofo@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 21:23:57]

Dry run: 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-07-21 22:09:48]

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

[commit-bot: I haz the power, 2016-07-21 22:09:49]

Dry run: This issue passed the CQ dry run.

[trizzofo, 2016-07-21 22:17:31]

The CQ bit was checked by trizzofo@google.com

[trizzofo, 2016-07-21 22:17:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
apacible@chromium.org, tommycli@chromium.org, tsepez@chromium.org,
avayvod@chromium.org, imcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2112673002/#ps160001
(title: "More refactoring on rebased code")

[commit-bot: I haz the power, 2016-07-21 22:18: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-07-21 22:25:49]

Description was changed from

==========
Removing references to Off the Record from comments and source code related to
Media Router

BUG=3333

R=tommycli@chromium.org
TBR=
==========

to

==========
Removing references to Off the Record from comments and source code related to
Media Router

BUG=3333

R=tommycli@chromium.org
TBR=
==========

[commit-bot: I haz the power, 2016-07-21 22:25:52]

Committed patchset #8 (id:160001)

[commit-bot: I haz the power, 2016-07-21 22:28:23]

Description was changed from

==========
Removing references to Off the Record from comments and source code related to
Media Router

BUG=3333

R=tommycli@chromium.org
TBR=
==========

to

==========
Removing references to Off the Record from comments and source code related to
Media Router

BUG=3333

R=tommycli@chromium.org
TBR=

Committed: https://crrev.com/9f8e900ba18c0e8a76b9b9e07dc51f0e9e4dcb72
Cr-Commit-Position: refs/heads/master@{#406968}
==========

[commit-bot: I haz the power, 2016-07-21 22:28:25]

Patchset 8 (id:??) landed as
https://crrev.com/9f8e900ba18c0e8a76b9b9e07dc51f0e9e4dcb72
Cr-Commit-Position: refs/heads/master@{#406968}

[trizzofo, 2016-07-22 00:37:14]

Description was changed from

==========
Removing references to Off the Record from comments and source code related to
Media Router

BUG=3333

R=tommycli@chromium.org
TBR=

Committed: https://crrev.com/9f8e900ba18c0e8a76b9b9e07dc51f0e9e4dcb72
Cr-Commit-Position: refs/heads/master@{#406968}
==========

to

==========
Removing references to Off the Record from comments and source code related to
Media Router

BUG=3333

R=tommycli@chromium.org
TBR=

Committed: https://crrev.com/9f8e900ba18c0e8a76b9b9e07dc51f0e9e4dcb72
Cr-Commit-Position: refs/heads/master@{#406968}
==========