811490 - Convert services/sync/tests/tps/test_privbrw_tabs.js to PB per window mode

Status: RESOLVED FIXED

(Testing Graveyard :: TPS, defect)

Description

[Andres Hernandez [:andreshm]]

Tabs engine have been updated to use PB per window. 

We need to convert services/sync/tests/tps/test_privbrw_tabs.js to handle PB per window.  

Follow up bug from Bug 722977 Comment 19.

Comment 1

[Andres Hernandez [:andreshm]]

Attachment: Patch v1

Comment 2

[(no longer active)]

Comment on attachment 682262 [details] [diff] [review]
Patch v1

Review of attachment 682262 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, although I'm not very familiar with the sync test code.

Comment 3

[Richard Newman [:rnewman]]

Comment on attachment 682262 [details] [diff] [review]
Patch v1

Review of attachment 682262 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me (there are some things I'd like to fix in tps.jsm, but I'm not going to r- code for following existing conventions!).

I'll file a follow-up to simplify and clean up some of TPS.

::: services/sync/tps/extensions/tps/modules/windows.jsm
@@ +5,5 @@
> + /* This is a JavaScript module (JSM) to be imported via
> +   Components.utils.import() and acts as a singleton.
> +   Only the following listed symbols will exposed on import, and only when
> +   and where imported. */
> +

Use strict in new code, please.

Comment 4

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/545cb0d6b5c6

Comment 5

[Ed Morley [:emorley]]

Push backed out for one of the changesets causing xpcshell failures of form:
https://tbpl.mozilla.org/php/getParsedLog.php?id=17110009&tree=Mozilla-Inbound

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d168b55e74ba

Comment 6

[(no longer active)]

Andres, can you please investigate the test failure?  Thanks!

Comment 7

[Andres Hernandez [:andreshm]]

The issue was not related to this patch. Was fixed in the latest patch on Bug 722977 Comment 24.

Comment 8

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9d2464e31941

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9d2464e31941

Comment 10

[Richard Newman [:rnewman]]

[phase3] Exception caught: Cannot modify properties of a WrappedNative Stack trace: TPS__SetPrivateBrowsing()@resource://tps/tps.jsm:688 < TPS.RunNextTestAction()@resource://tps/tps.jsm:481 < TPS__FinishAsyncOperation/<()@resource://tps/tps.jsm:154 <

Comment 11

[Richard Newman [:rnewman]]

Attachment: Follow-up. v1

Comment 12

[(no longer active)]

Please make sure to uplift this patch to Aurora as well.  Thanks!

Comment 13

[Richard Newman [:rnewman]]

(In reply to :Ehsan Akhgari from comment #12)
> Please make sure to uplift this patch to Aurora as well.  Thanks!

TPS doesn't run on Aurora, but sure :)

Comment 14

[(no longer active)]

(In reply to comment #13)
> (In reply to :Ehsan Akhgari from comment #12)
> > Please make sure to uplift this patch to Aurora as well.  Thanks!
> 
> TPS doesn't run on Aurora, but sure :)

Yeah, I'd just like to minimize the difference in the per-window PB related code on central and Aurora...

Comment 15

[Richard Newman [:rnewman]]

TPS passes now. Gonna land this on s-c without waiting for Greg's review.

Comment 16

[Richard Newman [:rnewman]]

https://hg.mozilla.org/services/services-central/rev/418277a17cb2

When this goes green, I'll merge and flag for approval.

Comment 17

[Richard Newman [:rnewman]]

TPS YEAAAAAHHHHH on s-c.

Landed on inbound, DONTBUILD:

https://hg.mozilla.org/integration/mozilla-inbound/rev/776cf96d305f

Comment 18

[Richard Newman [:rnewman]]

Comment on attachment 700014 [details] [diff] [review]
Follow-up. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Bug 722977 -- introducing per-window private browsing. Didn't catch all of the TPS Sync tests for private browsing.

User impact if declined: 
  None.

Testing completed (on m-c, etc.): 
  TPS green on s-c.

Risk to taking this patch (and alternatives if risky): 
  None. It's just to keep code aligned between Aurora and trunk; the patch that's now in Aurora is incomplete.

String or UUID changes made by this patch: 
  None.

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/776cf96d305f

Comment 20

[Alex Keybl [:akeybl]]

Comment on attachment 700014 [details] [diff] [review]
Follow-up. v1

https://bugzilla.mozilla.org/show_bug.cgi?id=804745#c37

Comment 21

[Richard Newman [:rnewman]]

Thanks, Alex.

https://hg.mozilla.org/releases/mozilla-aurora/rev/a2d668869033