381379 - remove G_Base64

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Robert Sayre]

We shouldn't need this anymore, now that atob and btoa are available to components. There are a few functions left over, but they can be replaced by array comprehensions. arrayifyString becomes [s.charCodeAt(i) for (i in s)].

Comment 2

[Robert Sayre]

zeniko, if you want this bug, feel free to take it

Comment 3

[Simon Bünzli]

Attachment: fix

(In reply to comment #0)
> arrayifyString becomes [s.charCodeAt(i) for (i in s)].

Unfortunately that doesn't work if String.prototype has been extended (which we should expect and effectively is the case for url-classifier). What works instead is Array.map(s, function(c) c.charCodeAt(0))

Comment 4

[:Gavin Sharp [email: [e-mail]]]

It'd be a good idea to convert the existing tests for the G_Base64 component to mochitests for the native atob/btoa functions (in a new bug).

Comment 5

[Tony Chang (Google)]

Attachment: fix syntax errors, make updateFromString pass through to update from stream

Patch to commit.

zeniko, have you been testing these patches?  This is the second patch with syntax errors and at least one other patch threw runtime errors.

Comment 6

[Tony Chang (Google)]

on trunk

Checking in browser/components/safebrowsing/content/application.js;
/cvsroot/mozilla/browser/components/safebrowsing/content/application.js,v  <--  application.js
new revision: 1.10; previous revision: 1.9
done
Checking in toolkit/components/url-classifier/content/enchash-decrypter.js;
/cvsroot/mozilla/toolkit/components/url-classifier/content/enchash-decrypter.js,v  <--  enchash-decrypter.js
new revision: 1.15; previous revision: 1.14
done
Checking in toolkit/components/url-classifier/content/url-crypto-key-manager.js;
/cvsroot/mozilla/toolkit/components/url-classifier/content/url-crypto-key-manager.js,v  <--  url-crypto-key-manager.js
new revision: 1.12; previous revision: 1.11
done
Checking in toolkit/components/url-classifier/content/url-crypto.js;
/cvsroot/mozilla/toolkit/components/url-classifier/content/url-crypto.js,v  <--  url-crypto.js
new revision: 1.6; previous revision: 1.5
done
Removing toolkit/components/url-classifier/content/moz/base64.js;
/cvsroot/mozilla/toolkit/components/url-classifier/content/moz/base64.js,v  <--  base64.js
new revision: delete; previous revision: 1.3
done
Checking in toolkit/components/url-classifier/content/moz/cryptohasher.js;
/cvsroot/mozilla/toolkit/components/url-classifier/content/moz/cryptohasher.js,v  <--  cryptohasher.js
new revision: 1.7; previous revision: 1.6
done
Checking in toolkit/components/url-classifier/src/nsUrlClassifierLib.js;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierLib.js,v  <--  nsUrlClassifierLib.js
new revision: 1.8; previous revision: 1.7
done
Checking in toolkit/components/url-classifier/src/nsUrlClassifierTable.js;
/cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierTable.js,v  <--  nsUrlClassifierTable.js
new revision: 1.7; previous revision: 1.6
done

Comment 7

[Tony Chang (Google)]

Bug 383261 filed about porting unittests.

Comment 8

[Simon Bünzli]

(In reply to comment #5)
> have you been testing these patches?

I'm testing the functional changes as far as possible. Since I haven't figured out yet how the whole fits together, there might be some code paths I've been missing. Sorry for those.

What was wrong with |function(c) c.charCodeAt(0)| though? I hope you're not referring to that - you'd just need a build with bug 381113 being fixed for testing (i.e. a Gecko 1.9a6pre one).

Comment 9

[Tony Chang (Google)]

You're right, this works on trunk builds.  I didn't test with enough context.