Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 6595500853690368: Issue 2172 - Parse URLs lazily on Safari to fix initlization on Safari <=6 (Closed)

Created:
March 19, 2015, 11:36 a.m. by Sebastian Noack
Modified:
March 20, 2015, 5:53 p.m.
Reviewers:
kzar
CC:
Wladimir Palant
Visibility:
Public.

Description

Issue 2172 - Parse URLs lazily on Safari to fix initlization on Safari <=6

Patch Set 1 : #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -2 lines) Patch
M safari/ext/background.js View 2 chunks +25 lines, -2 lines 2 comments Download

Messages

Total messages: 3
Sebastian Noack
March 19, 2015, 11:36 a.m. (2015-03-19 11:36:50 UTC) #1
kzar
LGTM http://codereview.adblockplus.org/6595500853690368/diff/5741031244955648/safari/ext/background.js File safari/ext/background.js (right): http://codereview.adblockplus.org/6595500853690368/diff/5741031244955648/safari/ext/background.js#newcode186 safari/ext/background.js:186: this._urlString = null; Nit: Shouldn't we do `delete ...
March 19, 2015, 3:35 p.m. (2015-03-19 15:35:09 UTC) #2
Sebastian Noack
March 19, 2015, 4:45 p.m. (2015-03-19 16:45:11 UTC) #3
http://codereview.adblockplus.org/6595500853690368/diff/5741031244955648/safa...
File safari/ext/background.js (right):

http://codereview.adblockplus.org/6595500853690368/diff/5741031244955648/safa...
safari/ext/background.js:186: this._urlString = null;
On 2015/03/19 15:35:10, kzar wrote:
> Nit: Shouldn't we do `delete this._urlString` here?

I try to be careful not to mess with the optimizer here.

Powered by Google App Engine
This is Rietveld