727309 - LocalBrowserDB needlessly computes URI + profile

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Richard Newman [:rnewman]]

The profile is private, and only set in the constructor.

Every call to appendProfile can turn into a member access. This will save a pile of garbage and a method call in almost every DB access.

Comment 1

[Richard Newman [:rnewman]]

Attachment: WIP. v1

This hasn't finished building yet, but otherwise shows my intent :)

Comment 2

[Lucas Rocha (:lucasr)]

Comment on attachment 597257 [details] [diff] [review]
WIP. v1

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

Makes sense.

::: mobile/android/base/db/LocalBrowserDB.java
@@ +79,5 @@
>  
>      private final String mProfile;
>      private long mMobileFolderId;
>  
> +    private final Uri bookmarksUriWithProfile;

Prefix all those members with 'm' for consistency.

Comment 3

[Richard Newman [:rnewman]]

Attachment: Proposed patch. v1

Rebased on top of current inbound. Nit addressed. One more method (introduced in the last day!) eliminated :)

Comment 4

[Lucas Rocha (:lucasr)]

Comment on attachment 597451 [details] [diff] [review]
Proposed patch. v1

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

Good.

::: mobile/android/base/db/LocalBrowserDB.java
@@ +92,5 @@
> +
> +        mBookmarksUriWithProfile = appendProfile(Bookmarks.CONTENT_URI);
> +        mParentsUriWithProfile   = appendProfile(Bookmarks.PARENTS_CONTENT_URI);
> +        mHistoryUriWithProfile   = appendProfile(History.CONTENT_URI);
> +        mImagesUriWithProfile    = appendProfile(Images.CONTENT_URI);

nit: we don't align assignments like this in Fennec :-) No alignment needed here.

Comment 5

[Richard Newman [:rnewman]]

Thanks Lucas.

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a2b70586e00

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/4a2b70586e00