Note: There are a few cases of duplicates in user autocompletion which are being worked on.

DOMStorage should obtain Private Browsing information from related docshell

RESOLVED FIXED in mozilla15

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jdm, Assigned: jdm)

Tracking

({addon-compat, dev-doc-needed})

Trunk
mozilla15
x86
Mac OS X
addon-compat, dev-doc-needed
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 12 obsolete attachments)

52.36 KB, patch
Details | Diff | Splinter Review
13.21 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
The global PB service is going away and the DOMStorageManager uses it to maintain a global state for consumers. I have a WIP patch that looks in the JS stack and finds the most recent related docshell, which sounds like it might work for content script.
(Assignee)

Updated

6 years ago
Depends on: 722840
(Assignee)

Comment 1

6 years ago
Created attachment 593343 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.
(Assignee)

Comment 2

6 years ago
Created attachment 593345 [details] [diff] [review]
Determine private browsing status dynamically from most recent docshell accessible WIP
(Assignee)

Comment 3

6 years ago
Comment on attachment 593345 [details] [diff] [review]
Determine private browsing status dynamically from most recent docshell accessible WIP

Honza, is this sane? Do you have any thoughts on a better way to accomplish this?
Attachment #593345 - Flags: feedback?(honzab.moz)
Comment on attachment 593345 [details] [diff] [review]
Determine private browsing status dynamically from most recent docshell accessible WIP

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

nsDOMStorageManager::InPrivateBrowsingMode() is used in both chrome and child process.  And also in IndexedDB code which I'm not familiar with.  I believe there will be cases you will access storage from a different context it was previously made for.

This won't be, as usual, that simple:

For DOM storage you will have to set the flag of the PB state directly on the storage instance when it is being created by 1) docshell in case of sessionStorage, 2) global window in case of localStorage and globalStorage (there is patch to remove GS very soon, so I wouldn't bother about it!).

You have to carry this flag to the parent process to let DOMStorageImpl know.  (the flag wont change during the instance(s) lifetime)

You have to change all calls to nsDOMStorageManager::gStorageManager->InPrivateBrowsingMode() in nsDOMStorageDBWrapper to query the flag on the storage, where possible.  Where not, probably assume PB:off, but maybe for more discussion or followups.

You have to ensure to always get rid of cached instances of sessionStorage/localStorage when PB mode is left for a docshell/global window.
Attachment #593345 - Flags: feedback?(honzab.moz) → feedback-
(Assignee)

Comment 5

6 years ago
Created attachment 598528 [details] [diff] [review]
Determine private browsing status dynamically from most recent docshell accessible WIP

Just checkpointing my work. I still need to figure out what to do in some of the nsDOMStorageWrapperDB methods.
(Assignee)

Updated

6 years ago
Attachment #593345 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Depends on: 729162
(Assignee)

Updated

6 years ago
Depends on: 725210
(Assignee)

Comment 6

6 years ago
Created attachment 599926 [details] [diff] [review]
Determine private browsing status dynamically from most recent docshell accessible WIP

This is close to being ready for review. I just need to fix up the content process handling for the observer notification and test what happens when the same storage is accessed by a private and non-private context concurrently.
(Assignee)

Updated

6 years ago
Attachment #598528 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Whiteboard: [needs updated patch]
(Assignee)

Updated

6 years ago
Depends on: 729204
(Assignee)

Comment 7

5 years ago
Created attachment 616892 [details] [diff] [review]
Determine private browsing status dynamically from most recent docshell accessible WIP
(Assignee)

Updated

5 years ago
Attachment #599926 - Attachment is obsolete: true
(Assignee)

Comment 8

5 years ago
Created attachment 616893 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.
(Assignee)

Updated

5 years ago
Attachment #593343 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
I still need to test concurrent access; these are just rebased and multiprocess-friendly.
(Assignee)

Comment 10

5 years ago
Created attachment 617027 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.
(Assignee)

Updated

5 years ago
Attachment #616893 - Attachment is obsolete: true
(Assignee)

Comment 11

5 years ago
Created attachment 617028 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.
(Assignee)

Updated

5 years ago
Attachment #617027 - Attachment is obsolete: true
(Assignee)

Comment 12

5 years ago
Comment on attachment 617028 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.

These tests ensure that IndexedDB is denied in private mode; that concurrent access to the same localStorage doesn't allow values to leak between privacy context; and that a simple enter and exit + set value test shows the correct behaviour.
Attachment #617028 - Flags: review?(honzab.moz)
(Assignee)

Comment 13

5 years ago
Comment on attachment 616892 [details] [diff] [review]
Determine private browsing status dynamically from most recent docshell accessible WIP

Here's the meat of the work.
Attachment #616892 - Flags: review?(honzab.moz)
(Assignee)

Updated

5 years ago
Assignee: nobody → josh
Whiteboard: [needs updated patch] → [needs review]
(Assignee)

Comment 14

5 years ago
Created attachment 617272 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.

Now with tests that pass as part of the overall test suite and not just when run standalone.
Attachment #617272 - Flags: review?(honzab.moz)
(Assignee)

Updated

5 years ago
Attachment #617028 - Attachment is obsolete: true
Attachment #617028 - Flags: review?(honzab.moz)
(Assignee)

Comment 15

5 years ago
Created attachment 617273 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.

Forgot a file.
Attachment #617273 - Flags: review?(honzab.moz)
(Assignee)

Updated

5 years ago
Attachment #617272 - Attachment is obsolete: true
Attachment #617272 - Flags: review?(honzab.moz)
Josh, I started the review.  Check bug 536509, it may collide (by means of need to merge) with your patches.
Comment on attachment 616892 [details] [diff] [review]
Determine private browsing status dynamically from most recent docshell accessible WIP

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

Doesn't look bad.

r=honzab with some details addressed.

::: dom/base/nsGlobalWindow.cpp
@@ +8262,5 @@
>        return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
>      }
> +
> +    nsCOMPtr<nsIPrivacyTransitionObserver> obs = do_QueryInterface(mSessionStorage);
> +   mDocShell->AddWeakPrivacyTransitionObserver(obs);

Indention

@@ +8312,5 @@
>  
> +    nsIDocShell* docShell = GetDocShell();
> +    nsCOMPtr<nsILoadContext> loadContext = do_QueryInterface(docShell);
> +    if (!loadContext) {
> +      return NS_OK;

Are we sure we want to exit here this way?  Pretend we are not in PB ? (sure, may be unsafe)

::: dom/indexedDB/CheckPermissionsHelper.cpp
@@ +95,3 @@
>      // TODO Support private browsing indexedDB?
>      return nsIPermissionManager::DENY_ACTION;
>    }

So, here when there is no load context, just pass through?  Contrast against what you do for localStorage.

::: dom/src/storage/StorageChild.cpp
@@ +276,5 @@
> +NS_IMETHODIMP
> +StorageChild::PrivateModeChanged(bool enabled)
> +{
> +  mInPrivateBrowsing = enabled;
> +  SendUpdatePrivateState(enabled);

Pity there isn't a central point for this.  There may be a lot of instances active and therefor a lot of messages.  Do you plan something like that?  (Not needed for this bug, though)

::: dom/src/storage/nsDOMStorage.cpp
@@ +820,5 @@
>    // If this is a cross-process situation, we don't have a real storage owner.
>    // All the correct checks have been done on the child, so we just need to
>    // make sure that our session-only status is correctly updated.
>    if (!mOwner)
> +    return nsDOMStorage::CanUseStorage(this, &mSessionOnly);

Hmm.. maybe just change the method to pass only |this| ?

@@ +1313,5 @@
>  
> +NS_IMETHODIMP
> +DOMStorageImpl::PrivateModeChanged(bool enabled)
> +{
> +  mInPrivateBrowsing = enabled;

Update also mSessionOnly.  Maybe just run CanUseStorage on |this|.
Attachment #616892 - Flags: review?(honzab.moz) → review+
Comment on attachment 617273 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.

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

Hard to track.

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_concurrent.js
@@ +3,5 @@
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +// Test opening two tabs that share a localStorage, but keep one in private mode.
> +// Ensure that values from one don't leak into the other, and that values from
> +// earlier private storage sessions aren't visible later.

Can you please list the steps one by one?  Like:
- in non private, set value=test
- in private check length=0 and value is null
- etc..

@@ +13,5 @@
> +  let browser = gBrowser.selectedBrowser;
> +  browser.addEventListener('load', function() {
> +    browser.removeEventListener('load', arguments.callee, true);
> +    gBrowser.selectedTab = gBrowser.addTab();
> +    let browser2 = gBrowser.selectedBrowser;

name there more descriptively like private_tab, non_private_tab.

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_localStorage_before_after.js
@@ +1,3 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */

Please add a comment what steps this test does, what external files it uses, and what is the overall purpose.
Comment on attachment 616892 [details] [diff] [review]
Determine private browsing status dynamically from most recent docshell accessible WIP

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

::: dom/interfaces/storage/nsIDOMStorageManager.idl
@@ +65,5 @@
>     * for a single origin.
>     */
>    nsIDOMStorage getLocalStorageForPrincipal(in nsIPrincipal aPrincipal,
> +                                            in DOMString aDocumentURI,
> +                                            in bool aPrivate);

Change IID!
(Assignee)

Comment 20

5 years ago
Created attachment 619891 [details] [diff] [review]
* * *

 - Determine private browsing status dynamically from most recent docshell accessible WIP
(Assignee)

Comment 21

5 years ago
Created attachment 619893 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.
(Assignee)

Updated

5 years ago
Attachment #617273 - Attachment is obsolete: true
Attachment #617273 - Flags: review?(honzab.moz)
(Assignee)

Comment 22

5 years ago
Comment on attachment 619893 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.

Now with more descriptive tests.
Attachment #619893 - Flags: review?(honzab.moz)
(Assignee)

Comment 23

5 years ago
Comment on attachment 619891 [details] [diff] [review]
* * *

This should be a pretty quick review; here's what I changed:

* addressed your comments
* made some tests pass by adding GetInterface nsDOMStorage[2] and switching to do_GetInterface in places that obtain an observer
* updated all call sites of getLocalStorageForPrincipal, thus making the remaining tests pass
Attachment #619891 - Flags: review?(honzab.moz)
(Assignee)

Comment 24

5 years ago
The change to getLocalStorageForPrincipal might affect addons. We should check what kind of use exists in mxr.mozilla.org/addons.
Keywords: addon-compat, dev-doc-needed

Comment 25

5 years ago
Try run for 7beb7938da34 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=7beb7938da34
Results (out of 131 total builds):
    success: 93
    warnings: 38
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/josh@joshmatthews.net-7beb7938da34
(In reply to Josh Matthews [:jdm] (travelling until June 25th, not reading non-CCed bugmail) from comment #24)
> The change to getLocalStorageForPrincipal might affect addons. We should
> check what kind of use exists in mxr.mozilla.org/addons.

I think we could make the second and the (new) third argument both optional.
Comment on attachment 619891 [details] [diff] [review]
* * *

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

Few comments left, all mostly nits.

r=honzab with that or arguments against.

::: dom/base/nsGlobalWindow.cpp
@@ +2546,5 @@
> +
> +    if (mLocalStorage) {
> +      nsCOMPtr<nsIPrivacyTransitionObserver> obs = do_GetInterface(mLocalStorage);
> +      if (obs) {
> +        mDocShell->AddWeakPrivacyTransitionObserver(obs);

Good catch, I didn't realize this the last time.

::: dom/indexedDB/CheckPermissionsHelper.cpp
@@ +92,5 @@
>    }
>  
> +  nsCOMPtr<nsIWebNavigation> webNav = do_GetInterface(aWindow);
> +  nsCOMPtr<nsILoadContext> loadContext = do_QueryInterface(webNav);
> +  if (loadContext && loadContext->UsePrivateBrowsing()) {

Do we have a test for this?

::: dom/interfaces/storage/nsIDOMStorageManager.idl
@@ +65,5 @@
>     * for a single origin.
>     */
>    nsIDOMStorage getLocalStorageForPrincipal(in nsIPrincipal aPrincipal,
> +                                            in DOMString aDocumentURI,
> +                                            in bool aPrivate);

Make both aPrivate and aDocumentURI optional.  JS compatibility is then preserved and you don't need to update existing tests as well.

::: dom/src/storage/nsDOMStorage.cpp
@@ +1441,3 @@
>  //static
>  bool
> +nsDOMStorage::CanUseStorage(DOMStorageBase* aStorage, bool* aSessionOnly)

Any reason why not to just pass aStorage?  (And default it to NULL)

You can then |if (aStorage) aStorage->mSessionOnly = ...|, simply just make the member friendly if it's not public already.

@@ +1802,5 @@
> +NS_IMETHODIMP
> +nsDOMStorage2::GetInterface(const nsIID & aIID, void **result)
> +{
> +  nsCOMPtr<nsIInterfaceRequestor> req =
> +    do_QueryInterface(NS_ISUPPORTS_CAST(nsIDOMStorageObsolete*, mStorage));

mStorage is nsDOMStorage, just do |mStorage->GetInterface(aIID, result);| directly?

::: dom/src/storage/nsDOMStorage.h
@@ +162,5 @@
>                                 nsAString& aOldValue) = 0;
>    virtual nsresult Clear(bool aCallerSecure, PRInt32* aOldCount) = 0;
> +
> +  // Call the above with |this| and |&this->mSessionOnly|
> +  bool CanUseStorage();

Not sure what "the above" is.  Otherwise, I like this method.
Attachment #619891 - Flags: review?(honzab.moz) → review+
Comment on attachment 619893 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.

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

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_concurrent.js
@@ +8,5 @@
> +
> +// Step 1: create new tab, load a page that sets test=value in non-private storage
> +// Step 2: create a new tab, load a page that sets test2=value2 in private storage
> +// Step 3: load a page in the tab from step 1 that checks the value of test2 and the total count in non-private storage
> +// Step 4: load a page in the tab from step 2 that checks the value of test and the total count in private storage

Step 3(,4): ... test2 *is value2* and the total count in non-private storage *is 1*

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_concurrent_page.html
@@ +10,5 @@
> +      aItKey = aCouples[nKeyId].split("=");  
> +      oGetVars[unescape(aItKey[0])] = aItKey.length > 1 ? unescape(aItKey[1]) : "";  
> +    }  
> +  }
> +  localStorage.length; // ensure all keys are cached

Is this necessary to make test pass?  If so, then it is a bug!

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_localStorage_before_after.js
@@ +29,5 @@
> +      let browser2 = gBrowser.selectedBrowser;
> +      gBrowser.removeTab(tab);
> +      browser2.addEventListener('load', function() {
> +        browser2.removeEventListener('load', arguments.callee, true);
> +        is(browser2.contentWindow.document.title, '1', 'localStorage should contain 1 item');

Hmm... so you set 'test' = 'value' in a PB tab and then you set again 'test' = 'value' in a non PB page, so if the value were already there you would just overwrite it.  The second load should better only check the value is not there (with getItem), right?  Or what is the purpose of this test?

In all your tests I don't initial and final localStorage cleanup.
Attachment #619893 - Flags: review?(honzab.moz)
(Assignee)

Updated

5 years ago
Attachment #616892 - Attachment is obsolete: true
(Assignee)

Comment 29

5 years ago
>::: dom/indexedDB/CheckPermissionsHelper.cpp
>@@ +92,5 @@
>>    }
>>  
>> +  nsCOMPtr<nsIWebNavigation> webNav = do_GetInterface(aWindow);
>> +  nsCOMPtr<nsILoadContext> loadContext = do_QueryInterface(webNav);
>> +  if (loadContext && loadContext->UsePrivateBrowsing()) {
>
>Do we have a test for this?

Yes. It's in the test patch with the existing indexeddb permission tests.

>In all your tests I don't initial and final localStorage cleanup.

I'm not sure what this means.
(In reply to Josh Matthews [:jdm] (travelling until June 25th) from comment #29)
> >In all your tests I don't initial and final localStorage cleanup.
> 
> I'm not sure what this means.

Sorry, has to be: In all your tests I don't *see* initial and final localStorage cleanup.
(Assignee)

Comment 31

5 years ago
What sort of init/cleanup are you looking for? I didn't see any happening in the localStorage mochitests.
Comment on attachment 619891 [details] [diff] [review]
* * *

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

::: dom/src/storage/nsDOMStorage.cpp
@@ +613,5 @@
> +{
> +  nsresult rv = mStorageImpl->QueryInterface(aIID, result);
> +  if (NS_SUCCEEDED(rv))
> +    return rv;
> +  return QueryInterface(aIID, result);;

Double ;;
(Assignee)

Comment 33

5 years ago
Created attachment 627173 [details] [diff] [review]
Determine private browsing status for DOMStorage from owning docshell if available, and receive updates if its privacy status changes.
(Assignee)

Updated

5 years ago
Attachment #619891 - Attachment is obsolete: true
(Assignee)

Comment 34

5 years ago
Created attachment 627174 [details] [diff] [review]
Basic sanity test for before/after private browsing DOM storage implementation.
(Assignee)

Updated

5 years ago
Attachment #619893 - Attachment is obsolete: true
(Assignee)

Comment 35

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/5a903d496ab0
http://hg.mozilla.org/integration/mozilla-inbound/rev/70cde80fa095
Target Milestone: --- → mozilla15
Sorry had to back out for crashes:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=70cde80fa095

eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=12062124&tree=Mozilla-Inbound

{
PROCESS-CRASH | file:///Users/cltbld/talos-slave/test/build/reftest/tests/content/base/crashtests/646184.html | application crashed (minidump found)
Crash dump filename: /var/folders/Hs/HsDn6a9SG8idoIya6p9mtE+++TI/-Tmp-/tmp3V6FL3/minidumps/5A7518E5-E1D7-4372-81A6-743E4C2A3723.dmp
Operating system: Mac OS X
                  10.6.8 10K549
CPU: amd64
     family 6 model 23 stepping 10
     2 CPUs

Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash address: 0x0

Thread 0 (crashed)
 0  XUL!nsGlobalWindow::GetLocalStorage [nsGlobalWindow.cpp : 8162 + 0x7]
    rbx = 0x00000000   r12 = 0x41f858a0   r13 = 0x00188ef0   r14 = 0x5fbfa280
    r15 = 0x41f85a68   rip = 0x02500407   rsp = 0x5fbfa0c0   rbp = 0x5fbfa1c0
    Found by: given as instruction pointer in context
 1  XUL!nsIDOMWindow_GetLocalStorage [dom_quickstubs.cpp : 503 + 0x4]
    rbx = 0x00000000   r12 = 0x09112290   r13 = 0x5fbfa4d0   r14 = 0x5fbfb390
    r15 = 0x5fbfa4c0   rip = 0x02c54a10   rsp = 0x5fbfa1d0   rbp = 0x5fbfa400
    Found by: call frame info
 2  XUL!js::CallJSPropertyOp [jscntxtinlines.h : 444 + 0xf]
    rbx = 0x00000000   r12 = 0x09112290   r13 = 0x5fbfb390   r14 = 0x5fbfa420
    r15 = 0x5fbfa4c0   rip = 0x03b914d0   rsp = 0x5fbfa410   rbp = 0x5fbfa460
    Found by: call frame info
 3  XUL!js_NativeGetInline [jsscopeinlines.h : 281 + 0x11]
    rbx = 0x5fbfa4c0   r12 = 0x25a60060   r13 = 0x5fbfa4d0   r14 = 0x25a5b070
    r15 = 0x09112290   rip = 0x03b6c790   rsp = 0x5fbfa470   rbp = 0x5fbfa510
    Found by: call frame info
 4  XUL!js_GetPropertyHelperInline [jsobj.cpp : 5087 + 0x11]
    rbx = 0x5fbfa600   r12 = 0x09112290   r13 = 0x5fbfb390   r14 = 0x25a60060
    r15 = 0x00000000   rip = 0x03b7098d   rsp = 0x5fbfa520   rbp = 0x5fbfa5d0
    Found by: call frame info
 5  XUL!js::Wrapper::get [jsobjinlines.h : 162 + 0x12]
    rbx = 0x25a5a060   r12 = 0x5fbfb390   r13 = 0x04b1eb30   r14 = 0x25a5f080
    r15 = 0x09112290   rip = 0x03c74ddb   rsp = 0x5fbfa5e0   rbp = 0x5fbfa660
    Found by: call frame info
 6  XUL!js::Proxy::get [jsproxy.cpp : 1090 + 0x20]
    rbx = 0x09112290   r12 = 0x25a5f080   r13 = 0x5fbfb390   r14 = 0x5fbfa680
    r15 = 0x080c7700   rip = 0x03bc877b   rsp = 0x5fbfa670   rbp = 0x5fbfa6d0
    Found by: call frame info
 7  XUL!proxy_GetGeneric [jsproxy.cpp : 1303 + 0xd]
    rbx = 0x25a5f080   r12 = 0x5fbfb390   r13 = 0x04b22d30   r14 = 0x25a4d2c0
    r15 = 0x09112290   rip = 0x03bc87b2   rsp = 0x5fbfa6e0   rbp = 0x5fbfa6e0
    Found by: call frame info
 8  XUL!js::Wrapper::get [jsobjinlines.h : 159 + 0xf]
    rbx = 0x25a5f080   r12 = 0x5fbfb390   r13 = 0x04b22d30   r14 = 0x25a4d2c0
    r15 = 0x09112290   rip = 0x03c74d9c   rsp = 0x5fbfa6f0   rbp = 0x5fbfa770
    Found by: call frame info
 9  XUL!js::CrossCompartmentWrapper::get [jswrapper.cpp : 525 + 0x38]
    rbx = 0x09112290   r12 = 0x25a4d2c0   r13 = 0x5fbfa790   r14 = 0x5fbfb390
    r15 = 0x04b22d30   rip = 0x03c77659   rsp = 0x5fbfa780   rbp = 0x5fbfa830
}


https://hg.mozilla.org/integration/mozilla-inbound/rev/f023c835794e
Target Milestone: mozilla15 → ---
Presume this is also related:
https://tbpl.mozilla.org/php/getParsedLog.php?id=12063725&tree=Mozilla-Inbound
(Assignee)

Updated

5 years ago
Blocks: 722990
(Assignee)

Comment 38

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/b1e2749bd72b
http://hg.mozilla.org/integration/mozilla-inbound/rev/0729990b395c

Try run was all green, so hopefully this will stick.
Whiteboard: [needs review]
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/b1e2749bd72b
https://hg.mozilla.org/mozilla-central/rev/0729990b395c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(In reply to Josh Matthews [:jdm] from comment #24)
> The change to getLocalStorageForPrincipal might affect addons. We should
> check what kind of use exists in mxr.mozilla.org/addons.

I'm getting about 40 matches in the MXR.

I see the additional parameter is optional. This mean that current users of this function won't break, right?
(Assignee)

Comment 41

5 years ago
Correct.
You need to log in before you can comment on or make changes to this bug.